[C] Diagnose declarations hidden in C++ (#137368)

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

Author: AaronBallman

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
 - Added ``-Wimplicit-int-enum-cast``, grouped under ``-Wc++-compat``, which
   diagnoses implicit conversion from integer types to an enumeration type in C,
   which is not compatible with C++. #GH37027

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.

clang/include/clang/Basic/DiagnosticGroups.td

@@ -156,13 +156,14 @@ 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 ImplicitIntToEnumCast : DiagGroup<"implicit-int-enum-cast",
                                       [ImplicitEnumEnumCast]>;
 def CXXCompat: DiagGroup<"c++-compat", [ImplicitVoidPtrCast, DefaultConstInit,
-                                        ImplicitIntToEnumCast]>;
+                                        ImplicitIntToEnumCast, HiddenCppDecl]>;
 
 def ExternCCompat : DiagGroup<"extern-c-compat">;
 def KeywordCompat : DiagGroup<"keyword-compat">;

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

clang/include/clang/Sema/Sema.h

@@ -3523,6 +3523,7 @@ class Sema final : public SemaBase {
   }
 
   void warnOnReservedIdentifier(const NamedDecl *D);
+  void warnOnCTypeHiddenInCPlusPlus(const NamedDecl *D);
 
   Decl *ActOnDeclarator(Scope *S, Declarator &D);
 

clang/lib/AST/DeclBase.cpp

@@ -1422,7 +1422,18 @@ 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;
+}
+
+bool DeclContext::LexicallyEncloses(const DeclContext *DC) const {
+  if (getPrimaryContext() != this)
+    return getPrimaryContext()->LexicallyEncloses(DC);
+
+  for (; DC; DC = DC->getLexicalParent())
+    if (!isa<LinkageSpecDecl, ExportDecl>(DC) &&
         DC->getPrimaryContext() == this)
       return true;
   return false;

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)
@@ -18864,6 +18894,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;

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