aaron.ballman added a comment.
Assuming we're back on the same page again, I think the only thing left in this
review is a commenting nit.
================
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:6846-6847
+ return;
+ if (hasBTFTagAttr(D, Str))
+ return;
+
----------------
yonghong-song wrote:
> 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?
ohhh, I think we weren't on the same page, sorry about that! I had the
semantics wrong in my head -- I was thinking we wanted to reject different
tags, but it's the opposite, we want to allow multiple tags so long as they
have different arguments. Assuming I have that correct now, this approach looks
correct to me (without diagnosing the ignored duplicates).
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
