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

fix #20000 by adding injected routines to toInject #20003

Closed
wants to merge 2 commits into from

Conversation

metagn
Copy link
Collaborator

@metagn metagn commented Jul 11, 2022

fixes #20000

Edit: Did not realize skipping CI for the second commit would stop the CI for the first commit, will force push with some fixes later

@metagn
Copy link
Collaborator Author

metagn commented Jul 11, 2022

Reason for CI failure: Basically, if you inject a routine, but also use a outside routine with the same name in the template, then that outside routine will not be captured at the template definition, and the user will instead need it imported. bind will be bypassed too. So using toInject isn't good enough.

Upon further inspection these lines are the relevant ones:

if i <= 1 and r != scForceOpen:
# XXX this makes more sense but breaks bootstrapping for now:
# (s.kind notin routineKinds or s.magic != mNone):
# for instance 'nextTry' is both in tables.nim and astalgo.nim ...
if not isField or sfGenSym notin s.flags:
result = newSymNode(s, info)
markUsed(c, info, s)
onUse(info, s)
else:
result = n

The XXX part is probably the correct fix but I am afraid to touch it for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Templates: Method call prefers injected proc to outside, normal call doesn't
1 participant