aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

Aside from changing the attribute name back to what it was (sorry about that), 
this LGTM!



================
Comment at: clang/include/clang/Basic/Attr.td:1474
+def ArmMveStrictPolymorphism : TypeAttr {
+  let Spellings = [Clang<"__clang_arm_mve_strict_polymorphism">];
+  let Documentation = [ArmMveStrictPolymorphismDocs];
----------------
simon_tatham wrote:
> aaron.ballman wrote:
> > Why does this have a `__clang` prefix? That seems a bit strange given that 
> > the attribute can be spelled 
> > `[[clang::__clang_arm_mve_strict_polymorphism]]`. I'd probably drop the 
> > `__clang_` prefix entirely.
> I was going by your recommendation for the spelling of `ArmMveAlias` in 
> D67159#1659296. I've taken the `__clang_` off this one; should I do the same 
> to that one as well, while I'm here?
> 
> (It ought to be safe to do that after the fact, because `ArmMveAlias` is 
> locked down so hard that surely it //can't// have any users except the 
> tablegen MveEmitter backend, which is easy to change.)
Oh, that's right, this is one of those "users should not write this themselves, 
it's only for compiler internals" attributes. I am sorry for the churn on this 
review, but we should probably keep the `__clang` here for consistency with the 
other attribute. Thank you for reminding me about the prior decision!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72518



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

Reply via email to