simon_tatham marked 2 inline comments as done. simon_tatham added a comment.
On the general discomfort with this attribute existing: I'd be happy to lock it down, or mark it as "not recommended" in some way, if that's any help. I don't personally intend any use of it outside a single system header file (namely `arm_mve.h`, which D67161 <https://reviews.llvm.org/D67161> will introduce the initial version of). A warning along the lines of "don't use this!", automatically suppressed by the usual change of warning settings in system headers, would seem like a perfectly reasonable precaution, for example. ================ Comment at: clang/include/clang/Basic/Attr.td:596 } +def ClangBuiltinOverride : Attr { + let Spellings = [GCC<"__clang_builtin">]; ---------------- aaron.ballman wrote: > Do you expect this attribute to be inherited on redeclarations? I suspect > this should be an `InheritableAttr`. > > Also, add a newline above it for visual separation, please. > > Finally, should this be a target-specific attribute so that it's only > available for your target, or do you expect this attribute to be used on all > target architectures? For my use case, I have no opinion about redeclarations: I expect to declare each affected function exactly once. If you think `InheritableAttr` is a more sensible default choice, I'm fine with that. Target-specific: I don't have a use case outside the ARM target, so I'd be happy to lock it down that way if you want. ================ Comment at: clang/include/clang/Basic/Attr.td:600 + let Subjects = SubjectList<[Function], ErrorDiag>; + let Documentation = [Undocumented]; +} ---------------- aaron.ballman wrote: > No new undocumented attributes, please. OK. I'd intended to leave it undocumented in order to discourage people from using it in any context //other// than a system header file. But fair enough – perhaps it should be documented even so. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67159/new/ https://reviews.llvm.org/D67159 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
