[PATCH] D106614: [Clang] add btf_tag attribute

[Yonghong Song via Phabricator via cfe-commits]

yonghong-song added inline comments.


================
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:6846-6847
+    return;
+  if (hasBTFTagAttr(D, Str))
+    return;
+
----------------
aaron.ballman wrote:
> This should diagnose that the attribute is being ignored due to the mismatch in tags (and probably have a note to the previous declaration as well so users can see both sides of the conflict).
Just to make sure we are on the same page. The attribute is ignored because it has been defined in the declaration. This is specifically to handle cases like:
    #define __tag1 __attribute__((btf_tag("info")))
    #define __tag2 __attribute__((btf_tag("info2")))
    int var __tag1 __tag1, __tag2;
The first __tag1 will be added to declaration successfully, but the second __tag1 will be ignored since there exists an identical one. __tag2 will be added to the declaration successfully.

I think handleBTFTagAttr() does not handle merging declarations? It is mergeBTFTagAttr below to handle merging? If handleBTFTagAttr() is to handle merging declarations, it will handle attributes the same as below mergeBTFTagAttr, i.e., merging all attributes at the same time doing dedup.

Do you want issue an warning for ignored attribute?


================
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:6853-6854
+BTFTagAttr *Sema::mergeBTFTagAttr(Decl *D, const BTFTagAttr &AL) {
+  if (hasBTFTagAttr(D, AL.getBTFTag()))
+    return nullptr;
+  return ::new (Context) BTFTagAttr(Context, AL, AL.getBTFTag());
----------------
aaron.ballman wrote:
> This should diagnose as well when ignoring the attribute.
Okay will do.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D106614/new/

https://reviews.llvm.org/D106614