yonghong-song added inline comments.
================
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;
----------------
aaron.ballman wrote:
> 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.)
Yes, I missed this one as typically I (and kernel developers) write the
attribute at the end of definition. I just checked that if we put attribute
before the struct name, when the attribute is handled/merged, it is actually an
incomplete definition.
================
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);
----------------
aaron.ballman wrote:
> 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.)
> ```
>
There are a little complication here. Maybe you can give some advice.
We could have code like
D1: void __tag1 __tag2 bar();
D2: void bar();
The reason is that we explicitly allow multiple btf_tag's in the same
declaration and this is exactly our use case.
Current merge*Attribute() framework will take one attribute at a time.
For example, first we will do
mergeBTFTagAttribute(D2, __tag1):
Here, we add __tag1 to D2, so we have
D2: void __tag1 bar();
we then do
mergeBTFTagAttribute(D2, __tag2):
but D2 already has a __tag1 which is incompatible with __tag2, and
we will issue an error here. But it is incorrect to issue an error
since the correct semantics is to inherit both __tag1 and __tag2 for D2.
Let me take a look at the code how to best handle such cases. Please also let
me know if you have any suggestions. thanks!
A little bit more. The following are possible cases to check:
(1) void bar();
void __tag1 __tag2 bar(); // fail
(2). void __tag1 bar();
void __tag1 __tag2 bar(); // fail
(3) void __tag1 __tag2 bar();
void __tag1 __tag2 bar(); // succeed
(4). void __tag1 __tag2 __tag3 bar();
void __tag1 __tag2 bar(); // fail
(5). void __tag3 bar();
void __tag1 __tag2 bar(); // fail
(6). void __tag1 __tag2 bar();
void bar() // succeed
Basically, for two same declaration except attributes, D1, D2,
no error/warning only if D2 has no btf_tag attributes, or
D1 and D2 have identical btf_tag attributes.
Do this sound reasonable?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D106614/new/
https://reviews.llvm.org/D106614
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
