Improve our handling of tag decls in function prototypes

r289225 broke AST invariants by reparenting enumerators into function
decl contexts. This improves things by only reparenting TagDecls while
also attempting to preserve the lexical declcontext chain. The
interesting example here is:
  int f(struct S { enum E { a = 1 } b; } c);

The semantic contexts of E and S should be f, and the lexical context of
S should be f and the lexical context of E should be S. We didn't do
that with r289225, but now we should.

This change should also improve our behavior on this example:
  void f() {
    extern void ext(struct S { } o);
    // S injected here
  }

Before r289225 we would only remove 'S' from the surrounding tag
injection context if it was the TU, but now we properly reparent S from
f to ext.

Fixes PR31366

llvm-svn: 289678

clang/lib/Sema/SemaDecl.cpp

@@ -7771,6 +7771,28 @@ static void checkIsValidOpenCLKernelParameter(
   } while (!VisitStack.empty());
 }
 
+/// Find the DeclContext in which a tag is implicitly declared if we see an
+/// elaborated type specifier in the specified context, and lookup finds
+/// nothing.
+static DeclContext *getTagInjectionContext(DeclContext *DC) {
+  while (!DC->isFileContext() && !DC->isFunctionOrMethod())
+    DC = DC->getParent();
+  return DC;
+}
+
+/// Find the Scope in which a tag is implicitly declared if we see an
+/// elaborated type specifier in the specified context, and lookup finds
+/// nothing.
+static Scope *getTagInjectionScope(Scope *S, const LangOptions &LangOpts) {
+  while (S->isClassScope() ||
+         (LangOpts.CPlusPlus &&
+          S->isFunctionPrototypeScope()) ||
+         ((S->getFlags() & Scope::DeclScope) == 0) ||
+         (S->getEntity() && S->getEntity()->isTransparentContext()))
+    S = S->getParent();
+  return S;
+}
+
 NamedDecl*
 Sema::ActOnFunctionDeclarator(Scope *S, Declarator &D, DeclContext *DC,
                               TypeSourceInfo *TInfo, LookupResult &Previous,
@@ -8247,15 +8269,37 @@ Sema::ActOnFunctionDeclarator(Scope *S, Declarator &D, DeclContext *DC,
     }
 
     if (!getLangOpts().CPlusPlus) {
-      // In C, find all the non-parameter declarations from the prototype and
-      // move them into the new function decl context as well. Typically they
-      // will have been added to the surrounding context of the prototype.
+      // In C, find all the tag declarations from the prototype and move them
+      // into the function DeclContext. Remove them from the surrounding tag
+      // injection context of the function, which is typically but not always
+      // the TU.
+      DeclContext *PrototypeTagContext =
+          getTagInjectionContext(NewFD->getLexicalDeclContext());
       for (NamedDecl *NonParmDecl : FTI.getDeclsInPrototype()) {
-        DeclContext *OldDC = NonParmDecl->getDeclContext();
-        if (OldDC->containsDecl(NonParmDecl))
-          OldDC->removeDecl(NonParmDecl);
-        NonParmDecl->setDeclContext(NewFD);
-        NewFD->addDecl(NonParmDecl);
+        auto *TD = dyn_cast<TagDecl>(NonParmDecl);
+
+        // We don't want to reparent enumerators. Look at their parent enum
+        // instead.
+        if (!TD) {
+          if (auto *ECD = dyn_cast<EnumConstantDecl>(NonParmDecl))
+            TD = cast<EnumDecl>(ECD->getDeclContext());
+        }
+        if (!TD)
+          continue;
+        DeclContext *TagDC = TD->getLexicalDeclContext();
+        if (!TagDC->containsDecl(TD))
+          continue;
+        TagDC->removeDecl(TD);
+        TD->setDeclContext(NewFD);
+        NewFD->addDecl(TD);
+
+        // Preserve the lexical DeclContext if it is not the surrounding tag
+        // injection context of the FD. In this example, the semantic context of
+        // E will be f and the lexical context will be S, while both the
+        // semantic and lexical contexts of S will be f:
+        //   void f(struct S { enum E { a } f; } s);
+        if (TagDC != PrototypeTagContext)
+          TD->setLexicalDeclContext(TagDC);
       }
     }
   } else if (const FunctionProtoType *FT = R->getAs<FunctionProtoType>()) {
@@ -12632,28 +12676,6 @@ static bool isAcceptableTagRedeclContext(Sema &S, DeclContext *OldDC,
   return false;
 }
 
-/// Find the DeclContext in which a tag is implicitly declared if we see an
-/// elaborated type specifier in the specified context, and lookup finds
-/// nothing.
-static DeclContext *getTagInjectionContext(DeclContext *DC) {
-  while (!DC->isFileContext() && !DC->isFunctionOrMethod())
-    DC = DC->getParent();
-  return DC;
-}
-
-/// Find the Scope in which a tag is implicitly declared if we see an
-/// elaborated type specifier in the specified context, and lookup finds
-/// nothing.
-static Scope *getTagInjectionScope(Scope *S, const LangOptions &LangOpts) {
-  while (S->isClassScope() ||
-         (LangOpts.CPlusPlus &&
-          S->isFunctionPrototypeScope()) ||
-         ((S->getFlags() & Scope::DeclScope) == 0) ||
-         (S->getEntity() && S->getEntity()->isTransparentContext()))
-    S = S->getParent();
-  return S;
-}
-
 /// \brief This is invoked when we see 'struct foo' or 'struct {'.  In the
 /// former case, Name will be non-null.  In the later case, Name will be null.
 /// TagSpec indicates what kind of tag this is. TUK indicates whether this is a

clang/test/CodeGen/decl-in-prototype.c

@@ -1,4 +1,4 @@
-// RUN: %clang -target i386-unknown-unknown -emit-llvm -S -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple i386-linux -debug-info-kind=limited -emit-llvm -o - %s | FileCheck %s
 
 const int AA = 5;
 
@@ -19,3 +19,8 @@ int f(void (*g)(), enum {AA,BB} h) {
     // CHECK: ret i32 0
     return AA;
 }
+
+// This used to crash with debug info enabled.
+int pr31366(struct { enum { a = 1 } b; } c) {
+  return a;
+}

clang/test/Sema/decl-in-prototype.c

@@ -52,3 +52,44 @@ void enum_in_fun_in_fun(void (*fp)(enum { AA, BB } e)) { // expected-warning {{w
   SA(1, AA == 5);
   SA(2, BB == 0);
 }
+
+void f7() {
+  extern void ext(struct S { enum E7 { a, b } o; } p); // expected-warning 2 {{will not be visible}}
+  ext(a); // expected-error {{use of undeclared identifier}}
+}
+
+int f8(struct S { enum E8 { a, b } o; } p) { // expected-warning 2 {{will not be visible}}
+  struct S o;
+  enum E8 x;
+  return a + b;
+}
+// expected-note@+1 {{forward declaration}}
+struct S o; // expected-error {{'struct S' that is never completed}}
+// expected-note@+1 {{forward declaration}}
+enum E8 x = a + b; // expected-error 2 {{undeclared identifier}} expected-error {{incomplete type 'enum E8'}}
+
+int f9(struct { enum e { a = 1 } b; } c) { // expected-warning {{will not be visible}}
+  return a;
+}
+
+int f10(
+  struct S { // expected-warning {{will not be visible}}
+    enum E10 { a, b, c } f; // expected-warning {{will not be visible}}
+  } e) {
+  return a == b;
+}
+
+int f11(
+  struct S { // expected-warning {{will not be visible}}
+    enum E11 { // expected-warning {{will not be visible}}
+      a, b, c
+    } // expected-warning {{expected ';' at end of declaration list}}
+  } // expected-error {{expected member name or ';'}}
+  e);
+
+void f12() {
+  extern int ext12(
+      struct S12 { } e // expected-warning {{will not be visible}}
+      );
+  struct S12 o; // expected-error {{incomplete type}} expected-note {{forward declaration}}
+}