-
Notifications
You must be signed in to change notification settings - Fork 160
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make structural copy handle Blists properly. #1455
Conversation
Obj copy; | ||
Obj tmp; | ||
|
||
copy = DoCopyBlist(list, mut); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is the declaration of the locals indented differently than the rest of the code? (For that matter: The other function you introduce indent by 2 spaces, this function by 4, mostly)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Insufficiently clever emacs settings. Will fix
@@ -334,7 +348,7 @@ void CleanBlistCopy ( | |||
Obj list ) | |||
{ | |||
/* remove the forwarding pointer */ | |||
ADDR_OBJ(list)[0] = ADDR_OBJ( ADDR_OBJ(list)[0] )[0]; | |||
ADDR_OBJ(list)[0] = ADDR_OBJ( ADDR_OBJ(list)[0] )[1]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this be ADDR_OBJ(list)[0] = ELM_PLIST( ADDR_OBJ(list)[0], 1 );
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes
@@ -312,7 +326,7 @@ Obj CopyBlistCopy ( | |||
Obj list, | |||
Int mut ) | |||
{ | |||
return ADDR_OBJ(list)[0]; | |||
return ADDR_OBJ(ADDR_OBJ(list)[0])[2]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this be return ELM_PLIST( ADDR_OBJ(list)[0], 2 );
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes
Codecov Report
@@ Coverage Diff @@
## master #1455 +/- ##
==========================================
+ Coverage 63.79% 63.8% +<.01%
==========================================
Files 1009 1009
Lines 337592 337601 +9
Branches 13483 13473 -10
==========================================
+ Hits 215375 215392 +17
+ Misses 119253 119247 -6
+ Partials 2964 2962 -2
|
@stevelinton do you plan on updating this PR? Or should somebody else take it over? |
This has been superseded by #1514. |
Addresses #1428