yonghong-song added inline comments.
================
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:
> yonghong-song wrote:
> > 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?
> > 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!
>
> Oh, that's... interesting! You want to accept multiple, potentially separate
> attribute specifiers on the initial declaration, but not allow any new
> attributes on redeclarations or the definition. Btw, there are some tests
> missing from your above comment. You also need to think about:
> ```
> void __attribute__((btf_tag("one"), btf_tag("two"))) bar(); // ok
> void bar(); // succeed
>
> void __attribute__((btf_tag("one"), btf_tag("two"))) bar(); // ok
> void __attribute__((btf_tag("one"), btf_tag("two"))) bar(); // succeed
>
> void __attribute__((btf_tag("one"), btf_tag("two"))) bar(); // ok
> void __attribute__((btf_tag("one"))) bar(); // fail
>
> void __attribute__((btf_tag("one"), btf_tag("two"))) bar(); // ok
> void __attribute__((btf_tag("one"), btf_tag("two"), btf_tag("three"))) bar();
> // fail
>
> [[clang::btf_tag("one")]] void bar [[clang::btf_tag("two")]] (); // ok
> void __attribute__((btf_tag("one"), btf_tag("two"))) bar(); // succeed
> ```
> I don't think we have any attributes that behave like that currently. Can you
> remind me: what is the concern if the attributes are additive on
> redeclaration?
>> Can you remind me: what is the concern if the attributes are additive on
>> redeclaration?
I guess my main concern is people may look at one place for attribute
availability. They may find it not there but actually it is defined in a
different places. But in practice this may not be an issue as people typically
will put attributes in the header *unique* declaration and/or the later
*unique* definition.
Let me just to use additive approach which will simplify the whole thing a lot.
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
