yonghong-song added a comment.

In D111199#3055928 <https://reviews.llvm.org/D111199#3055928>, @aaron.ballman 
wrote:

> In D111199#3054126 <https://reviews.llvm.org/D111199#3054126>, @yonghong-song 
> wrote:
>
>> @aaron.ballman Ping. This is to address your concern in D110127 
>> <https://reviews.llvm.org/D110127>, could you take a look whether my 
>> proposal for a new attribute btf_type_tag will be okay for you or not? 
>> Thanks!
>
> Thank you for the new approach! On the whole, I think this seems like the 
> better way to go.

Great. I will go with this approach then.

> One thing that's not clear to me is whether we expect to give this type 
> attribute any semantics beyond passing further information to debug info or 
> not. e.g., do we have to worry about things like:
>
>   #define __tag1 __attribute__((btf_type_tag("tag1")))
>   #define __tag2 __attribute__((btf_type_tag("tag2")))
>   
>   int * __tag1 t1;
>   int * __tag2 t2 = t1; // Diagnose a type mismatch?
>
> (I know you don't intend to support that sort of thing right now, but I'm 
> wondering about the future -- basically, I'm worried about the situation 
> where we accept code like that without a diagnostic now, but want to diagnose 
> it in the future and will force a breaking change on users.)

This is a good question. These type attributes are mostly for type checking. In 
linux kernel, they are used by sparse 
(https://www.kernel.org/doc/html/v4.11/dev-tools/sparse.html) for type 
checking. We could implement a clang phase to utilize such information for type 
checking as well. But this is not the goal so far. Even if we indeed 
implemented type checking based on btf_type_tag in the future, it is likely to 
issue as a warning instead of a hard error. The attribute is mostly used to 
compile linux kernel, so we should be okay, e.g., to fix all issues in the 
kernel before turning on type checking feature in clang, etc.

> You should definitely fix up the clang-format issues and fix identifiers to 
> meet the usual naming conventions, etc.

Yes, this is true. This is a RFC patch so I didn't do clang-format at all. Will 
make sure all clang-format issues fixed, naming conventions followed with the 
formal patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111199

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to