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

Mon Oct 11 11:52:40 PDT 2021

aaron.ballman added a comment.

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. 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.)

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

================
Comment at: clang/include/clang/AST/Type.h:4768
+private:
+  std::string BTFTypeTag;
+
----------------
I think we should be able to use a `StringRef` instead, rather than have to worry about allocations.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111199