aaron.ballman added inline comments.

================
Comment at: clang/include/clang/Basic/Attr.td:646
+def BuiltinAlias : Attr {
+  let Spellings = [Clang<"clang_builtin_alias">];
+  let Args = [IdentifierArgument<"BuiltinName">];
----------------
Hrm, this is interesting -- this will allow the user to write 
`__attribute__((clang_builtin_alias(...)))` which is nice, but it also allows 
`[[clang::clang_builtin_alias(...)]]` which duplicates the `clang` in that 
spelling and feels a bit awkward.

While we could steal `__attribute__((builtin_alias))` as it seems like no one 
is using it, that feels a bit heavy-handed as builtins may not be portable 
across compilers.

Based on that, I sort of think we should go with:
```
let Spellings = [CXX11<"clang", "builtin_alias">, C2x<"clang", 
"builtin_alias">, GNU<"clang_builtin_alias">];
```
What do you think?


================
Comment at: clang/include/clang/Basic/AttrDocs.td:4690
 
+def BuiltinAliasDocs : Documentation {
+  let Category = DocCatFunction;
----------------
Can you also update `ArmBuiltinAliasDocs` to point users to the more general 
attribute?


================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:4021
+def err_attribute_builtin_alias : Error<
+  "'clang_builtin_alias' attribute can only be applied to a ARM or RISC-V "
+  "builtin">;
----------------



================
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:5181
+      (!IsAArch64 && !IsARM && !IsRISCV)) {
+    S.Diag(AL.getLoc(), diag::err_attribute_builtin_alias);
+    return;
----------------



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100611

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

Reply via email to