Skip to content
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

Closed

Conversation

stevelinton
Copy link
Contributor

@stevelinton stevelinton commented Jun 26, 2017

Addresses #1428

Obj copy;
Obj tmp;

copy = DoCopyBlist(list, mut);
Copy link
Member

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)

Copy link
Contributor Author

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];
Copy link
Member

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 ); ?

Copy link
Contributor Author

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];
Copy link
Member

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 ); ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes

@codecov
Copy link

codecov bot commented Jun 26, 2017

Codecov Report

Merging #1455 into master will increase coverage by <.01%.
The diff coverage is 78.57%.

@@            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
Impacted Files Coverage Δ
src/blister.c 76.35% <78.57%> (+0.39%) ⬆️
src/objset.c 40.85% <0%> (-0.29%) ⬇️
src/hpc/thread.c 46.54% <0%> (-0.2%) ⬇️
src/listfunc.c 76.95% <0%> (+0.17%) ⬆️
hpcgap/lib/hpc/stdtasks.g 38.87% <0%> (+0.25%) ⬆️
src/funcs.c 69.94% <0%> (+0.28%) ⬆️
src/hpc/traverse.c 80.23% <0%> (+0.38%) ⬆️
lib/queue.g 69.59% <0%> (+3.19%) ⬆️

@fingolfin fingolfin added the kind: enhancement Label for issues suggesting enhancements; and for pull requests implementing enhancements label Jul 3, 2017
@fingolfin
Copy link
Member

@stevelinton do you plan on updating this PR? Or should somebody else take it over?

@markuspf
Copy link
Member

This has been superseded by #1514.

@markuspf markuspf closed this Jul 20, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: enhancement Label for issues suggesting enhancements; and for pull requests implementing enhancements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants