-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[Macros] Implement unqualified lookup for global macro-expanded peer declarations. #64065
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
Conversation
4909ee5
to
6e6ec02
Compare
@swift-ci please smoke test |
1 similar comment
@swift-ci please smoke test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is looking great!
lib/AST/Decl.cpp
Outdated
|
||
case MacroIntroducedDeclNameKind::Arbitrary: | ||
// Only freestanding and member macros can have arbitrary names. | ||
assert(role == MacroRole::Member || role == MacroRole::Declaration); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd rather not have this assert; ill-formed code could trigger this unless we're blocking this function from getting called on ill-formed code, and that feels dicey.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I replaced the assertion with a FIXME
for now, and I'm working on adding support for arbitrary
through this mechanism locally.
return nullptr; | ||
} | ||
|
||
void MacroDecl::getIntroducedNames(MacroRole role, ValueDecl *attachedTo, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this somehow report to its caller when arbitrary
is there?
// We do not need a fully resolved macro until expansion. Instead, we | ||
// conservatively consider peer names for all macro declarations with a | ||
// custom attribute name. Unqualified lookup for that name will later | ||
// invoke expansion of the macro, and will yield no results if the resolved | ||
// macro does not produce the requested name, so the only impact is possibly | ||
// expanding earlier than needed / unnecessarily looking in the top-level | ||
// auxiliary decl cache. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this approach a lot!
…rce lookup cache, which obviously can cause request cycles.
d554d13
to
57cca85
Compare
@swift-ci please smoke test |
This change adds
MacroDecl::getIntroducedNames
, which populates the given vector with a list of names introduced by the macro decl. This list of macro-covered names is used in unqualified lookup to lazily expand declarations with attached macros covering a requested name. This can also be used later for name validation of attached macros.Resolves: rdar://104345950