[Macros] Top-level freestanding macros #63553
Conversation
rxwei
requested review from
artemcm,
hborla,
xedin,
slavapestov,
ahoppen,
bnbarham,
CodaFi,
DougGregor,
rintaro,
zoecarver,
hyp and
egorzhdan
as code owners
rxwei
force-pushed
the
top-level-macro
branch
2 times, most recently
from
580b344 to
cd9763a
Compare
rxwei
force-pushed
the
top-level-macro
branch
4 times, most recently
from
a4b5316 to
a56e760
Compare
|
@swift-ci please smoke test |
There was a problem hiding this comment.
This is looking good! A few comments, but no major structural concerns. I'd like us to centralize the logic for "walk the flattened list" and use it everywhere, because there's a lot of ad hoc recursion over rewritten macros, and we're likely to need to expand that logic... that said, I'd be okay landing like this and addressing comments in subsequent pull requests, since this is a big chunk of work that moves freestanding macros forward considerably.
lib/Sema/TypeCheckExpr.cpp
Outdated
| // Note: Don't look into macro expansions as it would be a circular dependency | ||
| // to have macro arguments' default literal types depend on macro expansions. | ||
| auto lookup = TypeChecker::lookupUnqualified( | ||
| dc->getModuleScopeContext(), nameRef, SourceLoc(), |
There was a problem hiding this comment.
I wonder if we could conjure up a source location at the beginning of the current source file, so we get to rely on location-based lookup.
lib/Sema/TypeCheckMacros.cpp
Outdated
| SmallVector<ASTNode, 4> expansion(topLevelItems.begin(), | ||
| topLevelItems.end()); | ||
| for (auto &node : expansion) | ||
| if (auto *expr = node.dyn_cast<Expr *>()) |
There was a problem hiding this comment.
We should handle Stmt * here as well, right?
There was a problem hiding this comment.
I'll handle this in the next patch when I added support for codeItem.
|
@swift-ci please smoke test |
rxwei
force-pushed
the
top-level-macro
branch
4 times, most recently
from
9dc87ba to
e0b597b
Compare
|
@swift-ci please smoke test |
rxwei
force-pushed
the
top-level-macro
branch
2 times, most recently
from
a67dc47 to
1619b55
Compare
rxwei
force-pushed
the
top-level-macro
branch
3 times, most recently
from
f716fbc to
9cc0e02
Compare
rxwei
force-pushed
the
top-level-macro
branch
2 times, most recently
from
f4f3e6a to
d71be1e
Compare
rxwei
force-pushed
the
top-level-macro
branch
4 times, most recently
from
2c217d7 to
58c85d0
Compare
|
|
||
| for (const auto *macro : found->second) { | ||
| macro->getIntroducedNames(MacroRole::Declaration, | ||
| /*attachedTo*/ nullptr, |
There was a problem hiding this comment.
Nit: the dyn_cast<ValueDecl>(decl) above is already tolerant to MacroExpansionDecl, because the dynamic case will just return nullptr. I had intended for the code above to instead conditionalize the macro role and macro reference, instead of the entire if body.
Totally fine with a follow-up cleanup here, just wanted to mention it before I forget!
rxwei
force-pushed
the
top-level-macro
branch
2 times, most recently
from
a864d42 to
d24303c
Compare
|
@swift-ci please smoke test |
|
@swift-ci please smoke test |
|
@swift-ci please build toolchain |
|
@swift-ci please smoke test |
|
@swift-ci please build toolchain macos |
|
@swift-ci please smoke test macos |
Allow freestanding macros to be used at top-level.
#…asMacroExpansionDeclwhen we are not in scripting mode.MacroExpansionExpr, assign it a substituteMacroExpansionDeclif the macro reference resolves to a declaration macro. This doesn’t work quite fully yet and will be enabled in a future fix.