[Clang importer] Don't cache swift_attr source files that have CustomAttrs with arguments #77962
Conversation
DougGregor
requested review from
zoecarver,
hyp,
egorzhdan,
beccadax,
ian-twilightcoder and
Xazax-hun
as code owners
|
@swift-ci please smoke test |
lib/ClangImporter/ImportDecl.cpp
Outdated
| break; | ||
|
|
||
| for (auto attr : decl->getAttrs()) { | ||
| if (auto customAttr = dyn_cast<CustomAttr>(attr)) { |
There was a problem hiding this comment.
We probably want to also capture all subclasses with UNIMPLEMENTED_CLONE as well. Do we want the implementation of CustomAttr::clone() to assert if the argument list isn't empty?
There was a problem hiding this comment.
Yes, we can do that. Actually, I can avoid the clone here altogether with a different approach that's more similar to how member-attribute macros work.
DougGregor
requested review from
hborla,
slavapestov and
xedin
as code owners
…Attrs with arguments Since we can't do a proper "deep" clone of expression nodes, cloning such a CustomAttr is necessarily shallow. In such cases, don't cache the swift_attr source files at all, so we get fresh attribute nodes for each such usage.
…bute that won't work
DougGregor
force-pushed
the
importer-no-cache-custom-swift-attr
branch
from
f96c910 to
0c91a34
Compare
|
@swift-ci please smoke test |
| case DeclAttrKind::CLASS: \ | ||
| if (&CLASS##Attr::canClone == &DeclAttribute::canClone) \ | ||
| return true; \ | ||
| return static_cast<const CLASS##Attr *>(this)->canClone(); |
There was a problem hiding this comment.
Out of curiosity, why this manual dynamic dispatch rather than a virtual function? Does DeclAttribute not have a vtable or something?
There was a problem hiding this comment.
That's right: there's no vtable, so I don't want to add one.
|
@swift-ci please test Windows |
|
@swift-ci please smoke test Linux |
Since we can't do a proper "deep" clone of expression nodes, cloning such a CustomAttr is necessarily shallow. In such cases, don't cache the swift_attr source files at all, so we get fresh attribute nodes for each such usage.