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

remove inserted derefs for ref object fields when transforming to dot call #24498

Merged
merged 2 commits into from
Dec 4, 2024

Conversation

metagn
Copy link
Collaborator

@metagn metagn commented Dec 3, 2024

fixes #24492

Kind of a goofy way of doing this, but we count how many derefs were used for the first parameter before calling builtinFieldAccess, then count after, and if there are more now than before, we remove the added derefs. I thought maybe getting rid of #18298 would simplify it but maybe this would still be the best way.

For better encapsulation we could make dotTransformation take an nOrig param instead but this would be less efficient since it would need a copy, though semAsgn already makes one.

@Araq Araq added the merge_when_passes_CI mergeable once green label Dec 3, 2024
@ringabout ringabout merged commit 2529f33 into nim-lang:devel Dec 4, 2024
18 checks passed
Copy link
Contributor

github-actions bot commented Dec 4, 2024

Thanks for your hard work on this PR!
The lines below are statistics of the Nim compiler built from 2529f33

Hint: mm: orc; opt: speed; options: -d:release
177945 lines; 8.720s; 653.457MiB peakmem

narimiran pushed a commit that referenced this pull request Jan 14, 2025
… call (#24498)

fixes #24492

Kind of a goofy way of doing this, but we count how many derefs were
used for the first parameter before calling `builtinFieldAccess`, then
count after, and if there are more now than before, we remove the added
derefs. I thought maybe getting rid of #18298 would simplify it but
maybe this would still be the best way.

For better encapsulation we could make `dotTransformation` take an
`nOrig` param instead but this would be less efficient since it would
need a copy, though `semAsgn` already makes one.

(cherry picked from commit 2529f33)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge_when_passes_CI mergeable once green
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Invoking a procedure named the same as a ref object's field mentions the value object
3 participants