[PATCH] D111199: [POC][BTF] support btf_type_tag attribute

[Yonghong Song via Phabricator via cfe-commits]

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