[PATCH] [-fms-extensions] Permit 'override' in C++98 and 'sealed' as a synonym for 'final'

Richard Smith richard at metafoo.co.uk
Wed Oct 16 14:31:38 PDT 2013



================
Comment at: include/clang/Basic/Attr.td:399
@@ -399,1 +398,3 @@
+  let Spellings = [Keyword<"final">, Keyword<"sealed">];
+  let Args = [BoolArgument<"IsSpelledSealed">];
   let SemaHandler = 0;
----------------
Please instead reconstruct the spelling from the spelling list index on the attribute. Something like:

  let Accessors = [Accessor<"isSpelledAsSealed", [Keyword<"sealed">]>];

... would do the trick.

================
Comment at: lib/Parse/ParseDeclCXX.cpp:1840
@@ -1839,2 +1839,3 @@
       Ident_final = &PP.getIdentifierTable().get("final");
+      Ident_sealed = &PP.getIdentifierTable().get("sealed");
       Ident_override = &PP.getIdentifierTable().get("override");
----------------
Maybe only look this up in MicrosoftExt mode?

================
Comment at: lib/Parse/ParseDeclCXX.cpp:1847
@@ -1845,1 +1846,3 @@
 
+    if (II == Ident_sealed && getLangOpts().MicrosoftExt)
+      return VirtSpecifiers::VS_Sealed;
----------------
... and then drop the MicrosoftExt check here.

================
Comment at: lib/Parse/ParseDeclCXX.cpp:1881
@@ -1875,3 +1880,3 @@
         << VirtSpecifiers::getSpecifierName(Specifier);
-    } else {
+    } else if (!getLangOpts().MicrosoftExt) {
       Diag(Tok.getLocation(), getLangOpts().CPlusPlus11 ?
----------------
David Majnemer wrote:
> Reid Kleckner wrote:
> > We set -std=c++11 in clang-cl, so I don't think we need this.
> But not everyone uses clang-cl.
`MicrosoftExt` should not remove the warning for `final` being incompatible with c++98. Instead, please add an ext_<something> for uses of `sealed`, to indicate it's an MS extension.

================
Comment at: lib/Parse/ParseDeclCXX.cpp:1903-1909
@@ -1897,6 +1902,9 @@
     Ident_final = &PP.getIdentifierTable().get("final");
+    Ident_sealed = &PP.getIdentifierTable().get("sealed");
     Ident_override = &PP.getIdentifierTable().get("override");
   }
-  
-  return Tok.getIdentifierInfo() == Ident_final;
+
+  return Tok.getIdentifierInfo() == Ident_final ||
+         (Tok.getIdentifierInfo() == Ident_sealed &&
+          getLangOpts().MicrosoftExt);
 }
----------------
As before, please move the MicrosoftExt check earlier.

================
Comment at: lib/Parse/ParseDeclCXX.cpp:2548-2549
@@ -2531,1 +2547,4 @@
+      Diag(FinalLoc, getLangOpts().CPlusPlus11
+                         ? diag::warn_cxx98_compat_override_control_keyword
+                         : diag::ext_override_control_keyword)
         << "final";
----------------
Please warn on use of `sealed` here too.

================
Comment at: test/SemaCXX/MicrosoftExtensions.cpp:401
@@ +400,2 @@
+  virtual void OverrideMe() override;
+};
----------------
David Majnemer wrote:
> Reid Kleckner wrote:
> > I don't see a test for __is_sealed.
> Will do.
Also, please add a test showing that `sealed` prevents the class from being derived from / prevents the method from being overridden.


http://llvm-reviews.chandlerc.com/D1948



More information about the cfe-commits mailing list