ExtRepOfObj for IsLetterAssocWordRep: return mutable list #5805
Conversation
... as this is how GAP behaved for years, and also many (most?) other `ExtRepOfObj` implementations also return a mutable list. So it seems plausible to mimic that. At the same time, keep the return value of `ERepLettWord` immutable, to avoid many needless copies of a list in code that does things like `Length(ERepLettWord(w))` or `ERepLettWord(w)[2*n-1]`.
fingolfin
added
topic: library
release notes: not needed
There was a problem hiding this comment.
I agree with the argument that GAP did in fact return a mutable ExtRepOfObj result in this situation, and that there are packages which use this fact.
(Well, actually these packages were just lucky that they did not run into situations where the cache is used.)
I think that it is in general good that the mutability of results of ExtRepOfObj can depend on what the method in question does:
If just some internally stored data are returned then this object has to be immutable, and automatically making a copy (like in the current pull request) is unnecessary. And if an independent new object gets constructed by ExtRepOfObj, it is unnecessary to make it immutable.
If the return value is mutable then users know that it is safe to change it, if the return value is immutable then users who want to change it know that they have to make a copy first.
... as this is how GAP behaved for years, and also many (most?) other
ExtRepOfObjimplementations also return a mutable list. So it seems plausible to mimic that.At the same time, keep the return value of
ERepLettWordimmutable, to avoid many needless copies of a list in code that does things likeLength(ERepLettWord(w))orERepLettWord(w)[2*n-1].Addresses fallout from #5802 in an arguably better way. So
semigroupswouldn't have to change (CC @james-d-mitchell) andgroupoidscould roll back its changes (CC @cdwensley), though it would also be safe for it to stay as it is (though potentially a teeny tiny bit less efficient).Sorry for the back and forth folks.