martong marked an inline comment as done.
martong added a comment.
In D68634#1700591 <https://reviews.llvm.org/D68634#1700591>, @a_sidorin wrote:
> Hello Balasz,
> In my opinion, importing and setting the attributes should be handled by the
> stuff used in InitializeImportedDecl(). Can we extend it or reuse the code?
> It will allow us not to miss the required actions while importing a new
> function too.
I was thinking about that too, but it turned out to be a way more intrusive
change.
We have to work with the most recent existing decl (`PrevDecl`) of the
FunctionDecl whose attribute we currently import.
We can get the `PrevDecl` only with the help of the lookup. We can't get it
from the `ToD` param of `InitializeImportedDecl` because by the time when we
call `InitializeImportedDecl` the redecl chain is not connected. So, we should
pass `PrevDecl` then into `GetImportedOrCreateDecl`. It would require to
change all call expressions of `GetImportedOrCreateDecl` (58 occurences).
Besides, we would use the `PrevDecl` only in case of inheritable attributes,
only they require this special care. Note that there are a bunch of
non-inheritable attributes which are already handled correctly in
`InitializeImportedDecl`.
================
Comment at: clang/lib/AST/ASTImporter.cpp:7842
+// This implementation is inspired by Sema::mergeDeclAttributes.
+void ASTNodeImporter::CopyInheritedAttributes(FunctionDecl *Old,
+ FunctionDecl *New) {
----------------
shafik wrote:
> There are other attributes that [apply to functions as
> well](https://en.cppreference.com/w/cpp/language/attributes): `nodiscard`,
> `maybe_unused` and `deprecated`. Is there a reason not to support those as
> well?
Different attributes require different handling.
E.g. C++11 [[noreturn]] requires diagnostics if the first declaration of a
function does not specify a the attribute, but a later does:
```
void f();
[[noreturn]] void f(); // ERROR
```
If a function is declared with [[noreturn]] in one translation unit, and the
same function is declared without [[noreturn]] in another translation unit, the
program is ill-formed; no diagnostic required.
But this is not true to the GNU __attribute__((noreturn)).
Also, there are non-inheritable attributes, in their case we do not have to
copy anything from the existing most-recent-decl.
Thus, I think I am going to change the patch's name to indicate we deal only
with `__attribute__((noreturn))` and with `analyzer_noreturn`.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D68634/new/
https://reviews.llvm.org/D68634
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
