[C] Diagnose declarations hidden in C++

[AaronBallman]

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

[llvmbot]

@llvm/pr-subscribers-clang

Author: Aaron Ballman (AaronBallman)

Changes

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

---

Full diff: https://github.com/llvm/llvm-project/pull/137368.diff

8 Files Affected:

• (modified) clang/docs/ReleaseNotes.rst (+12)
• (modified) clang/include/clang/AST/DeclBase.h (+5-1)
• (modified) clang/include/clang/Basic/DiagnosticGroups.td (+4-1)
• (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+4)
• (modified) clang/include/clang/Sema/Sema.h (+1)
• (modified) clang/lib/AST/DeclBase.cpp (+11)
• (modified) clang/lib/Sema/SemaDecl.cpp (+33)
• (added) clang/test/Sema/decl-hidden-in-c++.c (+68)

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;
+};
+

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[erichkeane]

1 nit, that function could be cleaned up a bit, else LGTM.