[Sema] Diagnose more cases of static data members in local or unnamed classes

[john.brawn]

Summary

We currently diagnose static data members directly contained in unnamed classes, but we should also diagnose when they're in a class that is nested (directly or indirectly) in an unnamed class. Do this by iterating up the list of parent DeclContexts and checking if any is an unnamed class.

Similarly also check for function or method DeclContexts (which includes things like blocks and openmp captured statements) as then the class is considered to be a local class, which means static data members aren't allowed.

[john.brawn]

Adjusted to not give multiple errors when more than one error criteria is met.

[rjmccall]

clang/lib/Sema/SemaDecl.cpp

6916

This diagnostic actually ignores the tag kind that passed down to it, which should be fixed. Also, you should pass in the tag kind for the actual anonymous class you found.

While I'm looking at this code: isLocalClass is mis-designed and doesn't work for any of our non-FunctionDecl local contexts. This check should be if (RD->getParentFunctionOrMethod()).

[john.brawn]

Use the tag of the anonymous struct when emitting a diagnostic.

[john.brawn]

clang/lib/Sema/SemaDecl.cpp

6916

This diagnostic actually ignores the tag kind that passed down to it, which should be fixed. Also, you should pass in the tag kind for the actual anonymous class you found.

Will do.

While I'm looking at this code: isLocalClass is mis-designed and doesn't work for any of our non-FunctionDecl local contexts. This check should be if (RD->getParentFunctionOrMethod()).

CXXMethodDecl has FunctionDecl as a parent class, so isLocalClass will find and return a CXXMethodDecl. Checking it on

class Example {
  void method() {
    class X {
      static int x;
    };
  }
};

I get the error as expected. Do you have an example where it doesn't work?

[rjmccall]

clang/lib/Sema/SemaDecl.cpp

6916

Will do.

You need to also update the diagnostic to actually care about this.

Do you have an example where it doesn't work?

All of the standard C/C++ local contexts are FunctionDecls, but "blocks", ObjC methods, and OpenMP captured statements aren't. The simplest example would be (under -fblocks):

void test() {
  ^{
    class X {
      static int x;
    };
  }();
}

[john.brawn]

Detect more kinds of local class.

[john.brawn]

clang/lib/Sema/SemaDecl.cpp

6916

You need to also update the diagnostic to actually care about this.

It looks like it already does? E.g. in the tests added to anonymous-struct.cpp where the error is "anonymous struct" or "anonymous class" depending on if it's an anonymous class or struct.

[rjmccall]

Thanks, looks great.

clang/lib/Sema/SemaDecl.cpp

6916

Oh, sorry, I just misread the diagnostic.

clang/lib/Sema/SemaDecl.cpp

if (DC->isRecord() && !CurContext->isRecord()) {
         break;
       case SC_PrivateExtern:
         llvm_unreachable("C storage class in c++!");
       }
     }
 
     if (SC == SC_Static && CurContext->isRecord()) {
       if (const CXXRecordDecl *RD = dyn_cast<CXXRecordDecl>(DC)) {
-        // C++ [class.static.data]p2:
-        //   A static data member shall not be a direct member of an unnamed
-        //   or local class
-        // FIXME: or of a (possibly indirectly) nested class thereof.
-        if (RD->isLocalClass()) {
+        // Walk up the enclosing DeclContexts to check for any that are
+        // incompatible with static data members.
+        const DeclContext *FunctionOrMethod = nullptr;
+        const CXXRecordDecl *AnonStruct = nullptr;
+        for (DeclContext *Ctxt = DC; Ctxt; Ctxt = Ctxt->getParent()) {
+          if (Ctxt->isFunctionOrMethod()) {
+            FunctionOrMethod = Ctxt;
+            break;
+          }
+          const CXXRecordDecl *ParentDecl = dyn_cast<CXXRecordDecl>(Ctxt);
+          if (ParentDecl && !ParentDecl->getDeclName()) {
+            AnonStruct = ParentDecl;
+            break;
+          }
+        }
+        if (FunctionOrMethod) {
+          // C++ [class.static.data]p5: A local class shall not have static data
+          // members.
           Diag(D.getIdentifierLoc(),
                diag::err_static_data_member_not_allowed_in_local_class)
             << Name << RD->getDeclName() << RD->getTagKind();
-        } else if (!RD->getDeclName()) {
+        } else if (AnonStruct) {
+          // C++ [class.static.data]p4: Unnamed classes and classes contained
+          // directly or indirectly within unnamed classes shall not contain
+          // static data members.
           Diag(D.getIdentifierLoc(),
                diag::err_static_data_member_not_allowed_in_anon_struct)
-            << Name << RD->getTagKind();
+            << Name << AnonStruct->getTagKind();
           Invalid = true;
         } else if (RD->isUnion()) {
           // C++98 [class.union]p1: If a union contains a static data member,
           // the program is ill-formed. C++11 drops this restriction.
           Diag(D.getIdentifierLoc(),
                getLangOpts().CPlusPlus11
                  ? diag::warn_cxx98_compat_static_data_member_in_union
                  : diag::ext_static_data_member_in_union) << Name;
         }

clang/test/OpenMP/for_loop_messages.cpp

 }
 
 void test_nowait() {
 #pragma omp parallel
 #pragma omp for nowait nowait // expected-error {{directive '#pragma omp for' cannot contain more than one 'nowait' clause}}
   for (int i = 0; i < 16; ++i)
     ;
 }
+
+void test_static_data_member() {
+#pragma omp parallel
+#pragma omp for
+  for (int i = 0; i < 16; ++i) {
+    class X {
+      static int x; // expected-error {{static data member 'x' not allowed in local class 'X'}}
+    };
+  }
+}

clang/test/SemaCXX/anonymous-struct.cpp

 typedef struct {
   Empty E;
 } B;
 
 typedef struct {
   const Empty E;
 } C;
 } // namespace ImplicitDecls
+
+struct {
+  static int x; // expected-error {{static data member 'x' not allowed in anonymous struct}}
+} static_member_1;
+
+class {
+  struct A {
+    static int x; // expected-error {{static data member 'x' not allowed in anonymous class}}
+  } x;
+} static_member_2;
+
+union {
+  struct A {
+    struct B {
+      static int x; // expected-error {{static data member 'x' not allowed in anonymous union}}
+    } x;
+  } x;
+} static_member_3;

clang/test/SemaCXX/blocks.cpp

 
 namespace test7 {
 struct S {};
 void f() {
   constexpr S s;
   auto some_block = ^{ (void)s; };
 }
 }
+
+void static_data_member() {
+  auto block = ^{
+    class X {
+      static int x; // expected-error {{static data member 'x' not allowed in local class 'X'}}
+    };
+    class Y {
+      struct Z {
+        static int z; // expected-error {{static data member 'z' not allowed in local struct 'Z'}}
+      };
+    };
+  };
+}