https://github.com/AaronBallman updated https://github.com/llvm/llvm-project/pull/137368
>From e8234c9f374783f3b3fde586464037f52eda2f77 Mon Sep 17 00:00:00 2001 From: Aaron Ballman <aa...@aaronballman.com> Date: Fri, 25 Apr 2025 13:16:39 -0400 Subject: [PATCH 1/3] [C] Diagnose declarations hidden in C++ This introduces a new diagnostic, -Wc++-hidden-decl, which is grouped under -Wc++-compat, that diagnoses declarations which are valid in C but invalid in C++ due to the type being at the wrong scope. e.g., struct S { struct T { int x; } t; }; struct T t; // Valid C, invalid C++ This is implementing the other half of #21898 --- clang/docs/ReleaseNotes.rst | 12 ++++ clang/include/clang/AST/DeclBase.h | 6 +- clang/include/clang/Basic/DiagnosticGroups.td | 5 +- .../clang/Basic/DiagnosticSemaKinds.td | 4 ++ clang/include/clang/Sema/Sema.h | 1 + clang/lib/AST/DeclBase.cpp | 11 +++ clang/lib/Sema/SemaDecl.cpp | 33 +++++++++ clang/test/Sema/decl-hidden-in-c++.c | 68 +++++++++++++++++++ 8 files changed, 138 insertions(+), 2 deletions(-) create mode 100644 clang/test/Sema/decl-hidden-in-c++.c diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 6ecb97825ab8d..262c863c602d0 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -150,6 +150,18 @@ C Language Changes - Added ``-Wimplicit-void-ptr-cast``, grouped under ``-Wc++-compat``, which diagnoses implicit conversion from ``void *`` to another pointer type as being incompatible with C++. (#GH17792) +- Added ``-Wc++-hidden-decl``, grouped under ``-Wc++-compat``, which diagnoses + use of tag types which are visible in C but not visible in C++ due to scoping + rules. e.g., + + .. code-block:: c + + struct S { + struct T { + int x; + } t; + }; + struct T t; // Invalid C++, valid C, now diagnosed C2y Feature Support ^^^^^^^^^^^^^^^^^^^ diff --git a/clang/include/clang/AST/DeclBase.h b/clang/include/clang/AST/DeclBase.h index 2fb9d5888bce4..375e9e2592502 100644 --- a/clang/include/clang/AST/DeclBase.h +++ b/clang/include/clang/AST/DeclBase.h @@ -2239,10 +2239,14 @@ class DeclContext { return DC && this->getPrimaryContext() == DC->getPrimaryContext(); } - /// Determine whether this declaration context encloses the + /// Determine whether this declaration context semantically encloses the /// declaration context DC. bool Encloses(const DeclContext *DC) const; + /// Determine whether this declaration context lexically encloses the + /// declaration context DC. + bool LexicallyEncloses(const DeclContext *DC) const; + /// Find the nearest non-closure ancestor of this context, /// i.e. the innermost semantic parent of this context which is not /// a closure. A context may be its own non-closure ancestor. diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index 31e2cfa7ab485..47b24d5d6c112 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -154,10 +154,13 @@ def BuiltinRequiresHeader : DiagGroup<"builtin-requires-header">; def C99Compat : DiagGroup<"c99-compat">; def C23Compat : DiagGroup<"c23-compat">; def : DiagGroup<"c2x-compat", [C23Compat]>; +def HiddenCppDecl : DiagGroup<"c++-hidden-decl">; def DefaultConstInitUnsafe : DiagGroup<"default-const-init-unsafe">; def DefaultConstInit : DiagGroup<"default-const-init", [DefaultConstInitUnsafe]>; def ImplicitVoidPtrCast : DiagGroup<"implicit-void-ptr-cast">; -def CXXCompat: DiagGroup<"c++-compat", [ImplicitVoidPtrCast, DefaultConstInit]>; +def CXXCompat: DiagGroup<"c++-compat", + [ImplicitVoidPtrCast, DefaultConstInit, + HiddenCppDecl]>; def ExternCCompat : DiagGroup<"extern-c-compat">; def KeywordCompat : DiagGroup<"keyword-compat">; diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 7090cbe7acbe6..4be5654e6a63f 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -496,6 +496,10 @@ def warn_unused_lambda_capture: Warning<"lambda capture %0 is not " "%select{used|required to be captured for this use}1">, InGroup<UnusedLambdaCapture>, DefaultIgnore; +def warn_decl_hidden_in_cpp : Warning< + "%select{struct|union|enum}0 defined within a struct or union is not visible " + "in C++">, InGroup<HiddenCppDecl>, DefaultIgnore; + def warn_reserved_extern_symbol: Warning< "identifier %0 is reserved because %select{" "<ERROR>|" // ReservedIdentifierStatus::NotReserved diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 0c77c5b5ca30a..7bad162afa66f 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -3536,6 +3536,7 @@ class Sema final : public SemaBase { } void warnOnReservedIdentifier(const NamedDecl *D); + void warnOnCTypeHiddenInCPlusPlus(const NamedDecl *D); Decl *ActOnDeclarator(Scope *S, Declarator &D); diff --git a/clang/lib/AST/DeclBase.cpp b/clang/lib/AST/DeclBase.cpp index 1afda9aac9e18..d80bdeec092d0 100644 --- a/clang/lib/AST/DeclBase.cpp +++ b/clang/lib/AST/DeclBase.cpp @@ -1428,6 +1428,17 @@ bool DeclContext::Encloses(const DeclContext *DC) const { return false; } +bool DeclContext::LexicallyEncloses(const DeclContext* DC) const { + if (getPrimaryContext() != this) + return getPrimaryContext()->LexicallyEncloses(DC); + + for (; DC; DC = DC->getLexicalParent()) + if (!isa<LinkageSpecDecl>(DC) && !isa<ExportDecl>(DC) && + DC->getPrimaryContext() == this) + return true; + return false; +} + DeclContext *DeclContext::getNonTransparentContext() { DeclContext *DC = this; while (DC->isTransparentContext()) { diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index fe61b92e087d7..a71a6906febf6 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -6500,6 +6500,8 @@ NamedDecl *Sema::HandleDeclarator(Scope *S, Declarator &D, if (!New) return nullptr; + warnOnCTypeHiddenInCPlusPlus(New); + // If this has an identifier and is not a function template specialization, // add it to the scope stack. if (New->getDeclName() && AddToScope) @@ -15213,6 +15215,32 @@ void Sema::CheckFunctionOrTemplateParamDeclarator(Scope *S, Declarator &D) { } } +void Sema::warnOnCTypeHiddenInCPlusPlus(const NamedDecl *D) { + // This only matters in C. + if (getLangOpts().CPlusPlus) + return; + + // This only matters if the declaration has a type. + const auto *VD = dyn_cast<ValueDecl>(D); + if (!VD) + return; + + // Get the type, this only matters for tag types. + QualType QT = VD->getType(); + const auto *TD = QT->getAsTagDecl(); + if (!TD) + return; + + // Check if the tag declaration is lexically declared somewhere different + // from the lexical declaration of the given object, then it will be hidden + // in C++ and we should warn on it. + if (!TD->getLexicalParent()->LexicallyEncloses(D->getLexicalDeclContext())) { + unsigned Kind = TD->isEnum() ? 2 : TD->isUnion() ? 1 : 0; + Diag(D->getLocation(), diag::warn_decl_hidden_in_cpp) << Kind; + Diag(TD->getLocation(), diag::note_declared_at); + } +} + static void CheckExplicitObjectParameter(Sema &S, ParmVarDecl *P, SourceLocation ExplicitThisLoc) { if (!ExplicitThisLoc.isValid()) @@ -15334,6 +15362,8 @@ Decl *Sema::ActOnParamDeclarator(Scope *S, Declarator &D, New->setScopeInfo(S->getFunctionPrototypeDepth() - 1, S->getNextFunctionPrototypeIndex()); + warnOnCTypeHiddenInCPlusPlus(New); + // Add the parameter declaration into this scope. S->AddDecl(New); if (II) @@ -18860,6 +18890,9 @@ FieldDecl *Sema::CheckFieldDecl(DeclarationName Name, QualType T, if (InvalidDecl) NewFD->setInvalidDecl(); + if (!InvalidDecl) + warnOnCTypeHiddenInCPlusPlus(NewFD); + if (PrevDecl && !isa<TagDecl>(PrevDecl) && !PrevDecl->isPlaceholderVar(getLangOpts())) { Diag(Loc, diag::err_duplicate_member) << II; diff --git a/clang/test/Sema/decl-hidden-in-c++.c b/clang/test/Sema/decl-hidden-in-c++.c new file mode 100644 index 0000000000000..c967d4b474024 --- /dev/null +++ b/clang/test/Sema/decl-hidden-in-c++.c @@ -0,0 +1,68 @@ +// RUN: %clang_cc1 -fsyntax-only -verify -Wc++-hidden-decl %s +// RUN: %clang_cc1 -fsyntax-only -verify -Wc++-compat %s +// RUN: %clang_cc1 -fsyntax-only -verify=good %s +// RUN: %clang_cc1 -fsyntax-only -verify=cxx -x c++ -std=c++2c %s +// good-no-diagnostics + +struct A { + struct B { // #b-decl + int x; + } bs; + enum E { // #e-decl + One + } es; + int y; +}; + +struct C { + struct D { + struct F { // #f-decl + int x; + } f; + } d; +}; + +struct B b; // expected-warning {{struct defined within a struct or union is not visible in C++}} \ + expected-note@#b-decl {{declared here}} \ + cxx-error {{variable has incomplete type 'struct B'}} \ + cxx-note 3 {{forward declaration of 'B'}} +enum E e; // expected-warning {{enum defined within a struct or union is not visible in C++}} \ + expected-note@#e-decl {{declared here}} \ + cxx-error {{ISO C++ forbids forward references to 'enum' types}} \ + cxx-error {{variable has incomplete type 'enum E'}} \ + cxx-note 3 {{forward declaration of 'E'}} +struct F f; // expected-warning {{struct defined within a struct or union is not visible in C++}} \ + expected-note@#f-decl {{declared here}} \ + cxx-error {{variable has incomplete type 'struct F'}} \ + cxx-note {{forward declaration of 'F'}} + +void func(struct B b); // expected-warning {{struct defined within a struct or union is not visible in C++}} \ + expected-note@#b-decl {{declared here}} +void other_func(enum E e) { // expected-warning {{enum defined within a struct or union is not visible in C++}} \ + expected-note@#e-decl {{declared here}} \ + cxx-error {{variable has incomplete type 'enum E'}} + struct B b; // expected-warning {{struct defined within a struct or union is not visible in C++}} \ + expected-note@#b-decl {{declared here}} \ + cxx-error {{variable has incomplete type 'struct B'}} +} + +struct X { + struct B b; // expected-warning {{struct defined within a struct or union is not visible in C++}} \ + expected-note@#b-decl {{declared here}} \ + cxx-error {{field has incomplete type 'struct B'}} + enum E e; // expected-warning {{enum defined within a struct or union is not visible in C++}} \ + expected-note@#e-decl {{declared here}} \ + cxx-error {{field has incomplete type 'enum E'}} +}; + +struct Y { + struct Z1 { + int x; + } zs; + + struct Z2 { + // This is fine, it is still valid C++. + struct Z1 inner_zs; + } more_zs; +}; + >From 7df3587cd0e1d82a2f4c62ae0d116b5d17efa5d1 Mon Sep 17 00:00:00 2001 From: Aaron Ballman <aa...@aaronballman.com> Date: Mon, 28 Apr 2025 07:15:37 -0400 Subject: [PATCH 2/3] Fix formatting; NFC --- clang/lib/AST/DeclBase.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/lib/AST/DeclBase.cpp b/clang/lib/AST/DeclBase.cpp index d80bdeec092d0..c1a4fa2bc7af1 100644 --- a/clang/lib/AST/DeclBase.cpp +++ b/clang/lib/AST/DeclBase.cpp @@ -1428,7 +1428,7 @@ bool DeclContext::Encloses(const DeclContext *DC) const { return false; } -bool DeclContext::LexicallyEncloses(const DeclContext* DC) const { +bool DeclContext::LexicallyEncloses(const DeclContext *DC) const { if (getPrimaryContext() != this) return getPrimaryContext()->LexicallyEncloses(DC); >From d0ccfe774ca705a60249b3f0018a555ee2c17d12 Mon Sep 17 00:00:00 2001 From: Aaron Ballman <aa...@aaronballman.com> Date: Mon, 28 Apr 2025 13:49:26 -0400 Subject: [PATCH 3/3] Simplify; NFC --- clang/lib/AST/DeclBase.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clang/lib/AST/DeclBase.cpp b/clang/lib/AST/DeclBase.cpp index c1a4fa2bc7af1..fead99c5f28a9 100644 --- a/clang/lib/AST/DeclBase.cpp +++ b/clang/lib/AST/DeclBase.cpp @@ -1422,7 +1422,7 @@ bool DeclContext::Encloses(const DeclContext *DC) const { return getPrimaryContext()->Encloses(DC); for (; DC; DC = DC->getParent()) - if (!isa<LinkageSpecDecl>(DC) && !isa<ExportDecl>(DC) && + if (!isa<LinkageSpecDecl, ExportDecl>(DC) && DC->getPrimaryContext() == this) return true; return false; @@ -1433,7 +1433,7 @@ bool DeclContext::LexicallyEncloses(const DeclContext *DC) const { return getPrimaryContext()->LexicallyEncloses(DC); for (; DC; DC = DC->getLexicalParent()) - if (!isa<LinkageSpecDecl>(DC) && !isa<ExportDecl>(DC) && + if (!isa<LinkageSpecDecl, ExportDecl>(DC) && DC->getPrimaryContext() == this) return true; return false; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits