[PATCH] D148101: [clang] Ensure that Attr::Create(Implicit) chooses a valid syntax

Wed Apr 12 10:53:33 PDT 2023

erichkeane added inline comments.

================
Comment at: clang/include/clang/Basic/AttributeCommonInfo.h:55
+
+    /// The attibute has no source code manifestation and is only created
+    /// implicitly.
----------------
rsandifo-arm wrote:
> erichkeane wrote:
> > If I recall, there was some pretty awful funny business in some attributes, which would explicitly use '0' instead of AS_GNU as implicit.  Did you run into any of these?
> > 
> > Would it make sense to make AS_Implicit 'first' here to catch those?  Or perhaps make '0' ill-formed (and assert?) and make this '1'?
> Thanks for the reviews!
> 
> Bumping the values to 1 sounds good to me.  I've created https://reviews.llvm.org/D148148 for that.  I kept AS_GNU first due to:
> ```
>     // Note TableGen depends on the order above.  Do not add or change the order
>     // without adding related code to TableGen/ClangAttrEmitter.cpp.
> ```
> 
> (I don't know whether that still applies, but it seemed better to keep the tablegen-sensitive stuff at “one end” of the enum.)
Thanks!  I think that makes sense to me.  Just make sure the pre-commit CI still passes before committing this patch stack.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148101