[PATCH] D106614: [Clang] add btf_tag attribute

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang/include/clang/Basic/AttrDocs.td:2019
+targets. This attribute may be attached to a struct/union, struct/union field,
+function, function argument or variables declaration. If -g is specified,
+the ``ARGUMENT`` info will be preserved in IR and be emitted to dwarf.
----------------



================
Comment at: clang/include/clang/Basic/AttrDocs.td:2021
+the ``ARGUMENT`` info will be preserved in IR and be emitted to dwarf.
+For BPF target, the ``ARGUMENT`` info will be emitted to .BTF ELF section too.
+  }];
----------------



================
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:6841
+  if (auto *Rec = dyn_cast<RecordDecl>(D)) {
+    if (!Rec->isCompleteDefinition()) {
+      S.Diag(AL.getLoc(), diag::warn_btftag_attribute_fwd_decl_ignored) << Str;
----------------
Does this work for code like:
```
struct __attribute__((btf_tag(""))) S {
  int x;
};
```
(I didn't see a test case for the attribute being written in that location.)


================
Comment at: clang/test/Sema/attr-btf_tag.c:31
+
+int __tag2 foo(struct t1 *arg); // expected-warning {{attribute 'btf_tag("tag2")' ignored as not in later redeclaration or definition}}
+int __tag foo(struct t1 *arg);
----------------
This looks backwards to me -- I think the initial declaration of `foo()` is fine and shouldn't be diagnosed, it's the subsequent declarations of `foo()` with a different `btf_tag` argument that should be diagnosed.

I think additional test cases for these semantics is:
```
void bar();
void __tag bar(); // (I think I understood that you want this to diagnose.)

void __tag bar();
void bar(); // (But that you don't want this to diagnose.)
```



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106614