[Macros] Type-check attached macro arguments. #63154
Conversation
Type wrappers were invoking CustomAttrDeclRequest with a decl context of the attached nominal type rather than its decl context, so the request was invoked twice for the same custom attributes with different decl contexts.
invalid macro attributes in `findMacroForCustomAttr`.
hborla
requested review from
AnthonyLatsis,
slavapestov and
xedin
as code owners
|
@swift-ci please smoke test |
type checking macro attribute arguments into two different requests.
|
@swift-ci please smoke test |
| @@ -3590,7 +3590,7 @@ void ConstraintSystem::resolveOverload(ConstraintLocator *locator, | |||
| if (auto macro = dyn_cast<MacroDecl>(decl)) { | |||
| // Macro can only be used in an expansion. If we end up here, it's | |||
| // because we found a macro but are missing the leading '#'. | |||
| if (!locator->isForMacroExpansion()) { | |||
| if (!(locator->isForMacroExpansion() || locator->getAnchor().isImplicit())) { | |||
There was a problem hiding this comment.
Will this be replaced with a different locator check to cover attached macros, or... ?
There was a problem hiding this comment.
Yeah, I was going back and forth between generalizing MacroExpansionExpr to work for attached macros, or providing "this is an attached macro" through some other mechanism, e.g. a dedicated SolutionApplicationTarget::Kind. If we decide on a common approach for no-argument macros across attached and freestanding, I think generalizing MacroExpansionExpr makes the most sense.
| llvm::TinyPtrVector<ValueDecl *> macros; | ||
| findMacroForCustomAttr(attr, dc, macros); | ||
| if (!macros.empty()) | ||
| return nullptr; |
There was a problem hiding this comment.
Okay, so MacroOrNominalTypeDecl will go away again. Sorry for the detour.
| /// Resolve a given custom attribute to an attached macro declaration. | ||
| class ResolveAttachedMacroRequest | ||
| : public SimpleRequest<ResolveAttachedMacroRequest, | ||
| MacroDecl *(CustomAttr *, DeclContext *), |
There was a problem hiding this comment.
Should this return a ConcreteDeclRef that points at the MacroDecl, so we also have the inferred generic arguments? Otherwise, they won't be captured anywhere.
| @@ -17,7 +17,7 @@ | |||
| // REQUIRES: OS=macosx | |||
|
|
|||
| @attached(accessor) | |||
| macro myPropertyWrapper: Void = | |||
| macro myPropertyWrapper() = | |||
There was a problem hiding this comment.
I noted this offline, but we should consider this change in the context of expression macros as well. Perhaps macros should always have argument lists, but can be used without parentheses if all parameters have default arguments.
To type-check macro arguments, we build an overload set from all of the attached macro declarations found in
findMacroForCustomAttr, and then form aCallExprwith the overload set as the callee with the custom attribute arguments (or an implicit empty argument list). The concreteDeclRefExprwritten back to the type-checked AST is the resolved macro decl for the given arguments.Note that this change prevents macros from being declared using a
: Voidtype annotation, because we always build aCallExprto resolve the macro reference; a call to a macro declaration with typeVoidwould result in the errorcannot call value of non-function type 'Void'.