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

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@289678 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp
index 692853fd1463530a05513edb96373b34456f8300..89cb4398638f1a8393cfa503dbafd616b6b1f3be 100644 (file)
--- a/lib/Sema/SemaDecl.cpp
+++ b/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

diff --git a/test/CodeGen/decl-in-prototype.c b/test/CodeGen/decl-in-prototype.c
index 15efa6551bc38b707da2ae811b5b1ebb756d74b1..426adf4b765c45db7830e59b55e97deaf1fdae1e 100644 (file)
--- a/test/CodeGen/decl-in-prototype.c
+++ b/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;
+}

diff --git a/test/Sema/decl-in-prototype.c b/test/Sema/decl-in-prototype.c
index 36c13c0558d6439451f746818e845c2a73773466..64caea6a38a858207ccafbfe811e65ea21e245b0 100644 (file)
--- a/test/Sema/decl-in-prototype.c
+++ b/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}}
+}