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

Reid Kleckner rnk at google.com
Wed Sep 17 17:38:13 PDT 2014


================
Comment at: lib/Sema/SemaCXXScopeSpec.cpp:256-259
@@ +255,6 @@
+  CXXRecordDecl *RD = nullptr;
+  if (CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(S->getEntity()))
+    RD = MD->getParent();
+  else
+    RD = dyn_cast<CXXRecordDecl>(S->getEntity());
+
----------------
nikola wrote:
> rnk wrote:
> > I wonder if there is a better way to find the enclosing class scope.  What about this test case:
> >   struct B { typedef int T; };
> >   struct A : B { enum C { X = sizeof(__super::T) }; };
> You're right, but instead of handling this case I'd like to turn the assert bellow into an error. That will handle the lambdas as well. What do you think? This seems like an abuse to me :)
I think your Scope check will actually break on simple things like:
  struct B { typedef int T; };
  struct A : B { void f() { if (int x = 1) { return sizeof(__super::T); } } };

Scopes have a fairly close mapping to curly braces in the source level.

I think you can use something like:
  CXXRecordDecl *RD = nullptr;
  for (const Scope *S = getCurScope(); S; S = S->getParent()) {
    if (S->isFnScope()) {
      MD = dyn_cast<CXXMethodDecl>(S->getEntity());
      if (MD) RD = MD->getParent();
      break;
    } else if (S->isClassScope()) {
      RD = cast<CXXRecordDecl>(S->getEntity());
      break;
    }
  }
  if (!RD) error


================
Comment at: test/SemaCXX/MicrosoftExtensions.cpp:512
@@ +511,3 @@
+  }
+};
+
----------------
nikola wrote:
> rnk wrote:
> > Can you add some tests with more nested DeclContexts? enums form a DeclContext, so those are handy and can go inside records and functions.
> > 
> > You can also use inner classes like:
> >   struct Base { typedef int T; };
> >   struct Outer { struct Inner : Base { static const int x = sizeof(__super::T); }; };
> > 
> > I bet __super also misbehaves inside a lambda. I won't insist on actually handling this case in this patch, but we should at least have test coverage that shows we don't crash.
> This example works just fine, I'll add it to existing tests.
> 
> We're not crashing on lambdas but I'll add the tests. Currently we error out because lambda has no base classes but hopefully we can fix this by doing what I suggested in the previous comment.
> 
> msvc is so funny:
> 
> 
> ```
> void foo() {
>   []{ __super::bar(); };
> }
> ```
> 
> will fail with: 'super' : this keyword can only be used within the body of class member
> 
> 
> ```
> struct A {
>   void foo() {
>     []{ __super::bar(); };
>   }
> };
> ```
> 
> will fail with: illegal use of 'super': 'A' does not have any base classes
> 
> Adding a base class will change the error to: illegal use of 'super':  'A::foo::lambda_hash>' does not have any base classes
> 
> Have I missed any cases?
Nope, sounds good.

http://reviews.llvm.org/D4468






More information about the cfe-commits mailing list