[Swiftify] Adjust _SwiftifyImport invocation to align with the signature #81855
Conversation
of the macro in the currently laoded macro module Stdlib macros are shipped with the SDK, so we need to make sure that the way we invoke the macro matches the version of the macro in the SDK. This patch skips the `spanAvailability` parameter if it isn't present. There's no good way to test this, because we will always pick up the current macro in our llvm-lit test suite, but I've tested it manually. rdar://152278292
hnrklssn
requested review from
Xazax-hun,
zoecarver,
egorzhdan,
beccadax,
ian-twilightcoder,
j-hui,
fahadnayyar and
susmonteiro
as code owners
|
@swift-ci please smoke test |
| assert(SwiftifyImportDecl); | ||
| if (!SwiftifyImportDecl) | ||
| return false; | ||
| for (auto *Param : *SwiftifyImportDecl->parameterList) |
There was a problem hiding this comment.
Could this be replaced with llvm::find_if or something similar?
There was a problem hiding this comment.
It could be replaced with any_of, but it wouldn't really be any easier to read imo (nor more concise), and is a bit more annoying to debug with the jumping between stack frames
| bool hasMacroParameter(StringRef ParamName) { | ||
| ValueDecl *D = getDecl("_SwiftifyImport"); | ||
| auto *SwiftifyImportDecl = dyn_cast_or_null<MacroDecl>(D); | ||
| assert(SwiftifyImportDecl); |
There was a problem hiding this comment.
Could the compiler ever be used with on old enough SDK that does not have this macro? Or could the compiler be used without the SDK at all? If yes, maybe we don't want to assert here.
There was a problem hiding this comment.
If that's the case I think we have bigger issues: the macro will still be attached. This only handles whether to use the new or old signature. But it's a good point. @DougGregor what's our stance on using this with an SDK that doesn't contain the macro at all?
of the macro in the currently laoded macro module
Stdlib macros are shipped with the SDK, so we need to make sure that the way we invoke the macro matches the version of the macro in the SDK. This patch skips the
spanAvailabilityparameter if it isn't present. There's no good way to test this, because we will always pick up the current macro in our llvm-lit test suite, but I've tested it manually.rdar://152278292