[Macros] Unify MacroExpansionDecl/MacroExpansionExpr expansion logic #66296
Conversation
|
@swift-ci Please smoke test |
rintaro
changed the title
rintaro
requested review from
hborla,
xedin,
slavapestov,
ahoppen,
bnbarham,
CodaFi and
DougGregor
as code owners
| enum Kind { | ||
| kExpr, // MacroExpansionExpr. | ||
| kDecl, // MacroExpansionDecl. | ||
| }; |
There was a problem hiding this comment.
Wouldn’t an enum class be nicer here?
There was a problem hiding this comment.
Since I didn't want to pollute namespace of the subclasses, I hoisted this to global as FreestandingMacroKind.
include/swift/AST/PrettyStackTrace.h
Outdated
| @@ -93,7 +94,20 @@ class PrettyStackTraceAnyFunctionRef : public llvm::PrettyStackTraceEntry { | |||
| virtual void print(llvm::raw_ostream &OS) const override; | |||
| }; | |||
|
|
|||
| void printExprDescription(llvm::raw_ostream &out, Expr *E, | |||
| /// PrettyStackTraceAnyFreestandingMacroExpansion | |||
There was a problem hiding this comment.
This comment doesn’t seem to provide any value.
lib/Sema/TypeCheckMacros.cpp
Outdated
| if (!macroRoles.contains(MacroRole::Expression)) { | ||
| if (!macroRoles.contains(MacroRole::Declaration) && | ||
| !ctx.LangOpts.hasFeature(Feature::CodeItemMacros)) { | ||
| ctx.Diags.diagnose(loc, diag::macro_experimental, "code item", | ||
| "CodeItemMacros"); | ||
| return nullptr; | ||
| } |
There was a problem hiding this comment.
I don’t think the implementation matches the comment here. Declaration macros don’t emit a diagnostic.
AFAIK declaration macros are enabled by default now, so I think you should be able to simplify this to
| if (!macroRoles.contains(MacroRole::Expression)) { | |
| if (!macroRoles.contains(MacroRole::Declaration) && | |
| !ctx.LangOpts.hasFeature(Feature::CodeItemMacros)) { | |
| ctx.Diags.diagnose(loc, diag::macro_experimental, "code item", | |
| "CodeItemMacros"); | |
| return nullptr; | |
| } | |
| if (macroRoles.contains(MacroRole::CodeItem) && | |
| !ctx.LangOpts.hasFeature(Feature::CodeItemMacros)) { | |
| ctx.Diags.diagnose(loc, diag::macro_experimental, "code item", | |
| "CodeItemMacros"); | |
| return nullptr; | |
| } |
And yes, I just saw that you just copy-pasted this from the old implementation.
There was a problem hiding this comment.
Yeah I just moved this code. But let's do it in this PR. updated.
rintaro
force-pushed
the
macros-freestandingexpansion
branch
from
86fa585 to
a51ed72
Compare
|
@swift-ci Please smoke test |
'MacroExpansionDecl' and 'MacroExpansionExpr' have many common methods. Introduce a common base class 'FreestandingMacroExpansion' that holds 'MacroExpansionInfo'. Factor out common expansion logic to 'evaluateFreestandingMacro' function that resembles 'evaluateAttachedMacro'.
rintaro
force-pushed
the
macros-freestandingexpansion
branch
from
a51ed72 to
86d405b
Compare
|
@swift-ci Please smoke test |
1 similar comment
|
@swift-ci Please smoke test |
MacroExpansionDeclandMacroExpansionExprhave many common methods. Introduce a common base classFreestandingMacroExpansionthat holdsMacroExpansionInfo.Factor out common expansion logic to
evaluateFreestandingMacrofunction that resemblesevaluateAttachedMacro.