[PATCH] D134458: [AST] Add msvc-specific C++11 attributes

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Sep 22 12:17:58 PDT 2022


aaron.ballman added a comment.

Can you split this out into two separate reviews? That makes it easier for the reviewers so we don't mix up context as well as when we do code archeology for changes. (I also made some quick comments as I noticed things in the file, but I've not done a full review yet.)



================
Comment at: clang/include/clang/AST/Decl.h:2961
 
+  /// Determins whether this field has no unique address
+  bool hasNoUniqueAddress() const;
----------------



================
Comment at: clang/include/clang/Basic/Attr.td:3490
 
+def MSConstexpr : DeclOrTypeAttr {
+  let LangOpts = [MicrosoftExt];
----------------
Type attribute? That seems surprising -- this is probably meant to be `InheritableAttr`?


================
Comment at: clang/include/clang/Basic/Attr.td:3497
+
+def MSNoUniqueAddress : InheritableAttr, TargetSpecificAttr<TargetItaniumCXXABI> {
+  let LangOpts = [MicrosoftExt];
----------------
Hmmm, very odd that the msvc no unique address attribute is only supported on Itanium ABI targets. Should this be `TargetMicrosoftCXXABI` instead?


================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:3366-3368
+def warn_attribute_ignored_on_non_function :
+  Warning<"%0 attribute ignored on a non-function">,
+  InGroup<IgnoredAttributes>;
----------------
You shouldn't need to add this diagnostic -- if the `Subjects` list is correct in Attr.td, we automatically diagnose this sort of thing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134458



More information about the cfe-commits mailing list