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

Richard Smith richard at metafoo.co.uk
Thu Oct 17 16:02:04 PDT 2013


  LGTM with a couple of tweaks.


================
Comment at: lib/Parse/ParseDeclCXX.cpp:1883-1884
@@ -1876,1 +1882,4 @@
+    } else if (Specifier == VirtSpecifiers::VS_Sealed) {
+      Diag(Tok.getLocation(), diag::ext_ms_sealed_keyword)
+        << FixItHint::CreateReplacement(Tok.getLocation(), "final");
     } else {
----------------
Sadly I don't think we can give a fixit on this diagnostic: `final` and `sealed` are not semantically equivalent due to the behavior of `__is_sealed`, and we don't want `-fixit` mode to ever change the meaning of a valid program. (We don't follow the continue-as-if-fixit-were-applied rule here.) Either drop this or move it to a note.

================
Comment at: lib/Parse/ParseDeclCXX.cpp:2560-2564
@@ -2537,7 +2559,7 @@
 
-    // Parse any C++11 attributes after 'final' keyword.
-    // These attributes are not allowed to appear here,
-    // and the only possible place for them to appertain
-    // to the class would be between class-key and class-name.
-    CheckMisplacedCXX11Attribute(Attrs, AttrFixitLoc);
+          // Parse any C++11 attributes after 'final' keyword.
+          // These attributes are not allowed to appear here,
+          // and the only possible place for them to appertain
+          // to the class would be between class-key and class-name.
+          CheckMisplacedCXX11Attribute(Attrs, AttrFixitLoc);
   }
----------------
What's happened to the indentation here?

================
Comment at: lib/Sema/SemaDeclCXX.cpp:4413-4418
@@ -4408,6 +4412,8 @@
 
-  if (Record->isAbstract() && Record->hasAttr<FinalAttr>()) {
-    Diag(Record->getLocation(), diag::warn_abstract_final_class);
-    DiagnoseAbstractType(Record);
-  }
+  if (Record->isAbstract())
+    if (FinalAttr *FA = Record->getAttr<FinalAttr>()) {
+      Diag(Record->getLocation(), diag::warn_abstract_final_class)
+        << (FA->isSpelledAsSealed() ? "sealed" : "final");
+      DiagnoseAbstractType(Record);
+    }
 
----------------
Please add braces around this `if`.

================
Comment at: test/SemaCXX/MicrosoftExtensions.cpp:402
@@ +401,3 @@
+  // expected-error at +1 {{only virtual member functions can be marked 'sealed'}}
+  void SealedFunction() sealed;
+
----------------
Can I get a test for attempting to override a `sealed` function too?


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



More information about the cfe-commits mailing list