Changed non-destructive list-insert for 0th position to a desctructiv…#369
Open
Hunter11zolomon wants to merge 2 commits intoeuslisp:cl-compatiblefrom
Open
Changed non-destructive list-insert for 0th position to a desctructiv…#369Hunter11zolomon wants to merge 2 commits intoeuslisp:cl-compatiblefrom
Hunter11zolomon wants to merge 2 commits intoeuslisp:cl-compatiblefrom
Conversation
Affonso-Gui
requested changes
Mar 30, 2019
Member
Affonso-Gui
left a comment
There was a problem hiding this comment.
Thanks for your contribution.
However, there are some problems that should be fixed before merging:
- You modify a total of 5 files, but we only have interest in
common.l. Please remove the other 4 files from this PR (eusmap,include,Makefileandcommon.l~) - It would also be nice to have destructive effect for null lists (not only for zero pos).
nreverseis allowed to have arbitrary side effects. From CLHS:
(setq l (list 1 2 3)) => (1 2 3)
(nreverse l) => (3 2 1)
l => implementation-dependent
In eus it appears to be granted to have the list match the result, but code should not depend on this.
Lines 249 to 301 in 9cfd655
Author
|
Ok, Thanks I will look into the changes. And can you please elaborate on the side effects of nreverse? |
Member
In other words, the input may be destroyed as a way of optimizing the algorithm, having no guarantee that the result will correctly be stored in the input back again. In the case of sbcl, for example: CL-USER> (setq lst (list 1 2 3))
(1 2 3)
CL-USER> (nreverse lst)
(3 2 1)
CL-USER> lst
(1) |
Author
|
I will try to find a work around. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changed non-destructive list-insert for 0th position to a destructive one