[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