[PATCH] PR13236 - Microsoft compatibility: support __super specifier to access members of base classes

Reid Kleckner rnk at google.com
Thu Sep 25 09:36:32 PDT 2014


lgtm

Please commit, this looks pretty good. If you want to make any more substantial changes I'd rather see a new patch. Thanks for the feature!

>>! In D4468#15, @nikola wrote:
> Fingers crossed.
> 
> I'm not too happy with what I had to do with FileCheck to get the lambda test case working. This whole file expects c++11 to be disabled which prevents me from using -verify. I thought about moving this to MicrosoftCompatibility.cpp but that's misleading as __super is controlled by -mfs-extensions. Separate file for __super tests would be the way to solve all problems?

I don't see the FileCheck hacks in this patch. Did you add the file? That said, feel free to split the __super test cases out. MicrosoftExtensions.cpp dates back to a simpler time when there were fewer extensions. :)

================
Comment at: lib/Sema/SemaCXXScopeSpec.cpp:259
@@ +258,3 @@
+      CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(S->getEntity());
+      if (MD && !MD->isStatic())
+        RD = MD->getParent();
----------------
Why can't we use this in static member functions? It appears to work with MSVC 2013, and MSDN just says "can only appear inside a member function". This prints B::f:
  extern "C" void puts(const char *);
  struct B {
    static void f() {
      puts("B::f");
    }
  };
  struct A : B {
    static void f() {
      __super::f();
    }
  };
  int main() {
    A::f();
  }

================
Comment at: lib/Sema/SemaCXXScopeSpec.cpp:269
@@ +268,3 @@
+
+  if (!RD || RD->isLambda()) {
+    Diag(SuperLoc, diag::err_invalid_super_scope);
----------------
I think we should give a custom diagnostic for lambdas. The diagnostic we issue currently will confuse users, because it will appear that they are using __super from a method, and we will say they aren't.

================
Comment at: lib/Sema/TreeTransform.h:3106
@@ +3105,3 @@
+          cast_or_null<CXXRecordDecl>(getDerived().TransformDecl(
+              SourceLocation(), QNNS->getAsRecordDecl()));
+      SS.MakeSuper(SemaRef.Context, RD, Q.getBeginLoc(), Q.getEndLoc());
----------------
Use Q.getBeginLoc() instead of no SLoc?

================
Comment at: test/SemaCXX/MicrosoftExtensions.cpp:407-409
@@ +406,5 @@
+
+  // expected-error at +4 {{C++ requires a type specifier for all declarations}}
+  // expected-warning at +3 {{empty parentheses interpreted as a function declaration}}
+  // expected-note at +2 {{replace parentheses with an initializer to declare a variable}}
+  static void foo() {
----------------
Ouch, this is bad error recovery. It'd be nice to figure out why we can recover from 'Undeclared::foo();' with less spew.

http://reviews.llvm.org/D4468






More information about the cfe-commits mailing list