[cxx-interop] Import attributes on inherited C++ methods #59022
Conversation
|
|
||
| // CHECK: struct NonTrivial { | ||
| // CHECK-NEXT: init() | ||
| // CHECK-NEXT: @discardableResult |
There was a problem hiding this comment.
Seems like we're getting a lot of @discardableResult attrs when the base class' member didn't have that attribute. Is it possible that the base class had the attribute but it was false (so it didn't get printed)? Or do you have any idea what's going on here?
There was a problem hiding this comment.
I think clang importer attaches @discardableResult to all non-void methods that aren't annotated with __attribute__((warn_unused_result)) (see lib/ClangImporter/ImportDecl.cpp:9247). The reason they suddenly showed up in the test, is because I added the -print-implicit-attrs to check for attributes that are imported as implicit by the clang importer (like @available).
In order to make the test less cluttered, we could switch to using CHECK so we can ignore the lines with @discardableResult. Or I could add a new test for inherited attributes. What do you think?
There was a problem hiding this comment.
Ah, I see now. Thanks for explaining. Make sense. This LGTM.
| break; | ||
| } | ||
| case DAK_DiscardableResult: { | ||
| attrs.add(new (context) DiscardableResultAttr(true)); |
There was a problem hiding this comment.
I'm thinking this might not always be true, maybe that's why we're seeing all the attrs in the test below. But I don't know.
There was a problem hiding this comment.
I decided to always inherit the attributes as implicit because we also inherit methods as implicit. Do you think that makes sense?
There was a problem hiding this comment.
Yeah, I see now. Sounds good.
|
@swift-ci please test. |
|
@egorzhdan @skrtks OK for me to merge this? |
No objections from my side 👍 |
We now copy the
DeclAttributesthat are created when mapping Clang attributes to Swift attributes.Fixes #58460