[PATCH] D111199: [Clang][LLVM][Attr] support btf_type_tag attribute

[Aaron Ballman via Phabricator via llvm-commits]

aaron.ballman added inline comments.


================
Comment at: clang/lib/Sema/SemaType.cpp:6526
+  }
+
+  ASTContext &Ctx = S.Context;
----------------
yonghong-song wrote:
> aaron.ballman wrote:
> > Should you also validate that the type is a pointer type?
> Looks like we cannot do this. For example,
>    int __attribute__((btf_type_tag("tag"))) *p;
> 
> When we reach `HandleBTFTypeTagAttribute`, we actually have `Type` as `int`.
> We don't have information for its `parent` type.
> 
> During code generation, we have
>    pointer_type
>       attributed_type
>          builtin_int type
> at that point, we can check pointer type's next typeloc's to decide whether it has attributed_type or not.
Ugh, GNU-style attributes are particularly awful. My initial inclination was that's the exact sort of situation we want to ignore because the type attribute should appertain to the type it's specified on (int, not int *), but GNU-style attributes "slide" to whatever the compiler thinks makes the most sense, so they're unpredictable for these kinds of uses.

However, the rule of thumb in Clang is that ignoring the attribute should come with a diagnostic so the user knows it's being ignored, otherwise they have a harder time debugging problems when they've misused the attribute. Currently, I think this will give no diagnostics when ignoring the attribute in way too many situations (attaching it to a struct declaration, attaching it to a non-pointer, etc).

We could perhaps change `GetDeclSpecTypeForDeclarator()` to add additional checking logic after the call to `distributeTypeAttrsFromDeclarator()`. However, we don't currently have any type attributes that need this kind of special handling and so that could perhaps wait for a follow-up.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111199