prefer unambiguous captured symbols in symchoices #23104
Conversation
This file contains 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
metagn
force-pushed
the
preferred-sym
branch
from
52f8d3a
to
affa6c9
Compare
metagn
changed the title
metagn
commented
Dec 25, 2023
| previously the compiler would not allow other overloads in the instantiation | ||
| context to be used, which was not the case for 0 or more than 1 overloads. | ||
| Now other overloads are allowed with a preference for the original overload | ||
| in cases of ambiguity. Some code might need to be changed to use `bind` to |
There was a problem hiding this comment.
Since this can be breaking (required package PRs Nycto/DelaunayNim#5 and PMunch/nimlsp#168), we could make it experimental.
|
I don't like node flags in general, they always have quirky semantics (are they copied or not?) and are not part of the spec and not available in the macro system. If there is a way to introduce new node kinds instead, it's what we should do. |
|
There are 2 chief obstacles for making this and
This PR doesn't have the urgency that |
Araq
pushed a commit
that referenced
this pull request
Aug 25, 2024
…le types in symchoices (#23997) fixes #23898, supersedes #23966 and #23990 Since #20631 ambiguous type symbols in templates are rejected outright, now we generate a symchoice for type nodes if they're ambiguous, a generalization of what was done in #22375. This is done for generics as well. Symchoices also handle type symbols better now, ensuring their type is a `typedesc` type; this probably isn't necessary for everything to work but it makes the logic more robust. Similar to #23989, we have to prepare for the fact that ambiguous type symbols behave differently than normal type symbols and either error normally or relegate to other routine symbols if the symbol is being called. Generating a symchoice emulates this behavior, `semExpr` will find the type symbol first, but since the symchoice has other symbols, it will count as an ambiguous type symbol. I know it seems spammy to carry around an ambiguity flag everywhere, but in the future when we have something like #23104 we could just always generate a symchoice, and the symchoice itself would carry the info of whether the first symbol was ambiguous. But this could harm compiler performance/memory use, it might be better to generate it only when we have to, which in the case of type symbols is only when they're ambiguous.
narimiran
pushed a commit
that referenced
this pull request
Sep 16, 2024
…le types in symchoices (#23997) fixes #23898, supersedes #23966 and #23990 Since #20631 ambiguous type symbols in templates are rejected outright, now we generate a symchoice for type nodes if they're ambiguous, a generalization of what was done in #22375. This is done for generics as well. Symchoices also handle type symbols better now, ensuring their type is a `typedesc` type; this probably isn't necessary for everything to work but it makes the logic more robust. Similar to #23989, we have to prepare for the fact that ambiguous type symbols behave differently than normal type symbols and either error normally or relegate to other routine symbols if the symbol is being called. Generating a symchoice emulates this behavior, `semExpr` will find the type symbol first, but since the symchoice has other symbols, it will count as an ambiguous type symbol. I know it seems spammy to carry around an ambiguity flag everywhere, but in the future when we have something like #23104 we could just always generate a symchoice, and the symchoice itself would carry the info of whether the first symbol was ambiguous. But this could harm compiler performance/memory use, it might be better to generate it only when we have to, which in the case of type symbols is only when they're ambiguous. (cherry picked from commit 09dcff7)
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.
fixes #11184, fixes #15650, fixes (untested) #18785, cleans up after #20457, remainder of #22744 after split off from #23091
When a sym choice is being created, if the first symbol is not ambiguous in the current scope, mark it as
nfPreferredSym. Then when a sym choice has to be resolved and type disambiguation etc. is not enough, if the first symbol is markednfPreferredSymthen use it.This fixes the shortcoming of overloadable enums that required #20457 and allows us to implement #11184, now symbols that only have 1 overload in scope in generics/templates can still receive additional overloads in instantiation scope. However just generating an open sym choice with 1 element has issues:
falsefor example would have typeNonein generic contexts, which includes default parameter values in proc declarations (which callsemGenericStmt), which meansproc foo(param = false)would not be able to infer the type ofparam.nfSeminsemExprand also makes the compiler not callsemSym.The solution in this PR is to allow standalone sym nodes to also be annotated
nfPreferredSym, which makes them act the same as a unarynkOpenSymChoiceduring symbol lookup.