-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[Module-scope lookup] Find arbitrary names in macro expansions. #64968
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
Merged
DougGregor
merged 3 commits into
swiftlang:main
from
DougGregor:name-lookup-excluding-macros-in-macro-resolution
Apr 7, 2023
Merged
[Module-scope lookup] Find arbitrary names in macro expansions. #64968
DougGregor
merged 3 commits into
swiftlang:main
from
DougGregor:name-lookup-excluding-macros-in-macro-resolution
Apr 7, 2023
Conversation
This file contains hidden or 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
@swift-ci please smoke test |
hamishknight
reviewed
Apr 6, 2023
I feel like that's what I was hitting yesterday somewhat, among the various things I was trying... awesome to see progress on arbitrary name producing macros 👍 |
rxwei
approved these changes
Apr 6, 2023
…ments. Macros introduced a significant wrinkle into Swift's name lookup mechanism. Specifically, when resolving names (and, really, anything else) within the arguments to a macro expansion, name lookup must not try to expand any macros, because doing so trivially creates a cyclic dependency amongst the macro expansions that will be detected by the request-evaluator. Our lookup requests don't always have enough information to answer the question "is this part of an argument to a macro?", so we do a much simpler, more efficient, and not-entirely-sound hack based on the request-evaluator. Specifically, if we are in the process of resolving a macro (which is determined by checking for the presence of a `ResolveMacroRequest` in the request-evaluator stack), then we adjust the options used for the name lookup request we are forming to exclude macro expansions. The evaluation of that request will then avoid expanding any macros, and not produce any results that involve entries in already-expanded macros. By adjusting the request itself, we still distinguish between requests that can and cannot look into macro expansions, so it doesn't break caching for those immediate requests. Over time, we should seek to replace this heuristic with a location-based check, where we use ASTScope to determine whether we are inside a macro argument. This existing check might still be useful because it's going to be faster than a location-based query, but the location-based query can be fully correct. This addresses a class of cyclic dependencies that we've been seeing with macros, and aligns the lookup behavior for module-level lookups with that specified in the macros proposals. It is not fully complete because lookup until nominal types does not yet support excluding results from macro expansions.
Whenever we perform a name lookup, we need to make sure to expand all macros that use `names: arbitrary`, because of course they can produce... wait for it... arbitrary names, and we don't know what they are until we instatiate them.
…ypes/extensions Eliminate a source of cyclic dependencies by not expanding macros when we are resolving macro arguments within a type or extension context. This extends the scheme introduced for module-scope lookup to also apply to lookup within types.
7338e00
to
527a4cd
Compare
@swift-ci please smoke test |
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.
Whenever we perform a name lookup, we need to make sure to expand
all macros that use
names: arbitrary
, because of course they canproduce... wait for it... arbitrary names, and we don't know what they
are until we instantiate them. This makes both freestanding and peer
macros at module scope work with arbitrary names.
However, enabling this lookup introduces a huge number of cyclic dependencies
to the compilation model, so teach module-scope lookup to avoid expanding macros
when we're resolving the arguments of a macro. This is a significant piece of the
model that we hadn't implement yet, and is crucial for unlocking the above.
More details follow.
Macros introduced a significant wrinkle into Swift's name lookup mechanism.
Specifically, when resolving names (and, really, anything else) within the
arguments to a macro expansion, name lookup must not try to expand any
macros, because doing so trivially creates a cyclic dependency amongst the
macro expansions that will be detected by the request-evaluator.
Our lookup requests don't always have enough information to answer the
question "is this part of an argument to a macro?", so we do a much simpler,
more efficient, and not-entirely-sound hack based on the request-evaluator.
Specifically, if we are in the process of resolving a macro (which is
determined by checking for the presence of a
ResolveMacroRequest
in therequest-evaluator stack), then we adjust the options used for the name
lookup request we are forming to exclude macro expansions. The evaluation
of that request will then avoid expanding any macros, and not produce any
results that involve entries in already-expanded macros. By adjusting the
request itself, we still distinguish between requests that can and cannot
look into macro expansions, so it doesn't break caching for those immediate
requests.
Over time, we should seek to replace this heuristic with a location-based
check, where we use ASTScope to determine whether we are inside a macro
argument. This existing check might still be useful because it's going to
be faster than a location-based query, but the location-based query can be
fully correct.
This addresses a class of cyclic dependencies that we've been seeing
with macros, and aligns the lookup behavior for module-level lookups
with that specified in the macros proposals. It is not fully complete
because lookup until nominal types does not yet support excluding
results from macro expansions.