[PATCH] D80295: [Sema] Diagnose more cases of static data members in local or unnamed classes
This revision was automatically updated to reflect the committed changes.
Closed by commit rG6c906f7785da: [Sema] Diagnose more cases of static data
members in local or unnamed classes (authored by john.brawn).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D80295/new/
https://reviews.llvm.org/D80295
Files:
clang/lib/Sema/SemaDecl.cpp
clang/test/OpenMP/for_loop_messages.cpp
clang/test/SemaCXX/anonymous-struct.cpp
clang/test/SemaCXX/blocks.cpp
Index: clang/test/SemaCXX/blocks.cpp
===
--- clang/test/SemaCXX/blocks.cpp
+++ clang/test/SemaCXX/blocks.cpp
@@ -153,3 +153,16 @@
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'}}
+ };
+};
+ };
+}
Index: clang/test/SemaCXX/anonymous-struct.cpp
===
--- clang/test/SemaCXX/anonymous-struct.cpp
+++ clang/test/SemaCXX/anonymous-struct.cpp
@@ -153,3 +153,21 @@
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;
Index: clang/test/OpenMP/for_loop_messages.cpp
===
--- clang/test/OpenMP/for_loop_messages.cpp
+++ clang/test/OpenMP/for_loop_messages.cpp
@@ -831,3 +831,13 @@
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'}}
+};
+ }
+}
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -6885,18 +6885,34 @@
if (SC == SC_Static && CurContext->isRecord()) {
if (const CXXRecordDecl *RD = dyn_cast(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(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,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D80295: [Sema] Diagnose more cases of static data members in local or unnamed classes
rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.
Thanks, looks great.
Comment at: clang/lib/Sema/SemaDecl.cpp:6904
<< Name << RD->getTagKind();
Invalid = true;
+} else if (RD->isLocalClass()) {
john.brawn wrote:
> rjmccall wrote:
> > john.brawn wrote:
> > > rjmccall wrote:
> > > > 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())`.
> > > > 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?
> > > 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;
> > };
> > }();
> > }
> > ```
> > 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.
Oh, sorry, I just misread the diagnostic.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D80295/new/
https://reviews.llvm.org/D80295
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D80295: [Sema] Diagnose more cases of static data members in local or unnamed classes
john.brawn marked an inline comment as done.
john.brawn added inline comments.
Comment at: clang/lib/Sema/SemaDecl.cpp:6904
<< Name << RD->getTagKind();
Invalid = true;
+} else if (RD->isLocalClass()) {
rjmccall wrote:
> john.brawn wrote:
> > rjmccall wrote:
> > > 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())`.
> > > 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?
> > 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;
> };
> }();
> }
> ```
> 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.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D80295/new/
https://reviews.llvm.org/D80295
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D80295: [Sema] Diagnose more cases of static data members in local or unnamed classes
john.brawn updated this revision to Diff 265779.
john.brawn retitled this revision from "[Sema] Diagnose static data members in
classes nested in unnamed classes" to "[Sema] Diagnose more cases of static
data members in local or unnamed classes".
john.brawn edited the summary of this revision.
john.brawn added a comment.
Herald added a reviewer: jdoerfert.
Herald added a subscriber: sstefan1.
Detect more kinds of local class.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D80295/new/
https://reviews.llvm.org/D80295
Files:
clang/lib/Sema/SemaDecl.cpp
clang/test/OpenMP/for_loop_messages.cpp
clang/test/SemaCXX/anonymous-struct.cpp
clang/test/SemaCXX/blocks.cpp
Index: clang/test/SemaCXX/blocks.cpp
===
--- clang/test/SemaCXX/blocks.cpp
+++ clang/test/SemaCXX/blocks.cpp
@@ -153,3 +153,16 @@
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'}}
+ };
+};
+ };
+}
Index: clang/test/SemaCXX/anonymous-struct.cpp
===
--- clang/test/SemaCXX/anonymous-struct.cpp
+++ clang/test/SemaCXX/anonymous-struct.cpp
@@ -153,3 +153,21 @@
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;
Index: clang/test/OpenMP/for_loop_messages.cpp
===
--- clang/test/OpenMP/for_loop_messages.cpp
+++ clang/test/OpenMP/for_loop_messages.cpp
@@ -831,3 +831,13 @@
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'}}
+};
+ }
+}
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -6885,18 +6885,34 @@
if (SC == SC_Static && CurContext->isRecord()) {
if (const CXXRecordDecl *RD = dyn_cast(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(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,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
