Reapply r151638 and r151641.

The bug that was caught by Apple's internal buildbots was valid and also showed another bug in my implementation.

These are now fixed, with regression tests added to catch them both (not Darwin-specific).

Original log:
====================

Revert r151638 because it causes assertion hit on PCH creation for Cocoa.h

Original log:
---------------------
Correctly track tags and enum members defined in the prototype of a function, and ensure they are properly scoped.

This fixes code such as:

enum e {x, y};
int f(enum {y, x} n) {
 return 0;
}

This finally fixes PR5464 and PR5477.
---------------------

I also reverted r151641 which was enhancement on top of r151638.

====================




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

16 files changed, 206 insertions, 6 deletions

diff --git a/include/clang/AST/Decl.h b/include/clang/AST/Decl.h
index c99ad115cf..1338657ff4 100644
--- a/include/clang/AST/Decl.h
+++ b/include/clang/AST/Decl.h
@@ -1431,6 +1431,11 @@ private:
   /// no formals.
   ParmVarDecl **ParamInfo;
 
+  /// DeclsInPrototypeScope - Array of pointers to NamedDecls for
+  /// decls defined in the function prototype that are not parameters. E.g.
+  /// 'enum Y' in 'void f(enum Y {AA} x) {}'.
+  llvm::ArrayRef<NamedDecl*> DeclsInPrototypeScope;
+
   LazyDeclStmtPtr Body;
 
   // FIXME: This can be packed into the bitfields in Decl.
@@ -1796,6 +1801,11 @@ public:
     setParams(getASTContext(), NewParamInfo);
   }
 
+  const llvm::ArrayRef<NamedDecl*> &getDeclsInPrototypeScope() const {
+    return DeclsInPrototypeScope;
+  }
+  void setDeclsInPrototypeScope(llvm::ArrayRef<NamedDecl *> NewDecls);
+
   /// getMinRequiredArguments - Returns the minimum number of arguments
   /// needed to call this function. This may be fewer than the number of
   /// function parameters, if some of the parameters have default
diff --git a/include/clang/Basic/DiagnosticGroups.td b/include/clang/Basic/DiagnosticGroups.td
index 7b45a9a71c..f175b39d70 100644
--- a/include/clang/Basic/DiagnosticGroups.td
+++ b/include/clang/Basic/DiagnosticGroups.td
@@ -244,6 +244,7 @@ def VectorConversions : DiagGroup<"vector-conversions">;      // clang specific
 def VexingParse : DiagGroup<"vexing-parse">;
 def VLA : DiagGroup<"vla">;
 def VolatileRegisterVar : DiagGroup<"volatile-register-var">;
+def Visibility : DiagGroup<"visibility">;
 
 // GCC calls -Wdeprecated-writable-strings -Wwrite-strings.
 def GCCWriteStrings : DiagGroup<"write-strings" , [DeprecatedWritableStr]>;
diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index c347c2e002..78d1d8d3d4 100644
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -132,7 +132,11 @@ def warn_unused_variable : Warning<"unused variable %0">,
 def warn_unused_exception_param : Warning<"unused exception parameter %0">,
   InGroup<UnusedExceptionParameter>, DefaultIgnore;
 def warn_decl_in_param_list : Warning<
-  "declaration of %0 will not be visible outside of this function">;
+  "declaration of %0 will not be visible outside of this function">,
+  InGroup<Visibility>;
+def warn_redefinition_in_param_list : Warning<
+  "redefinition of %0 will not be visible outside of this function">,
+  InGroup<Visibility>;
 def warn_empty_parens_are_function_decl : Warning<
   "empty parentheses interpreted as a function declaration">,
   InGroup<VexingParse>;
diff --git a/include/clang/Sema/Scope.h b/include/clang/Sema/Scope.h
index 9b3dd9299d..e9aa173cb9 100644
--- a/include/clang/Sema/Scope.h
+++ b/include/clang/Sema/Scope.h
@@ -297,6 +297,10 @@ public:
   /// \brief Determine whether this scope is a C++ 'try' block.
   bool isTryScope() const { return getFlags() & Scope::TryScope; }
 
+  /// containedInPrototypeScope - Return true if this or a parent scope
+  /// is a FunctionPrototypeScope.
+  bool containedInPrototypeScope() const;
+
   typedef UsingDirectivesTy::iterator udir_iterator;
   typedef UsingDirectivesTy::const_iterator const_udir_iterator;
 
diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h
index 499ee31d4a..d7ff5491f8 100644
--- a/include/clang/Sema/Sema.h
+++ b/include/clang/Sema/Sema.h
@@ -902,6 +902,17 @@ public:
   // Symbol table / Decl tracking callbacks: SemaDecl.cpp.
   //
 
+  /// List of decls defined in a function prototype. This contains EnumConstants
+  /// that incorrectly end up in translation unit scope because there is no
+  /// function to pin them on. ActOnFunctionDeclarator reads this list and patches
+  /// them into the FunctionDecl.
+  std::vector<NamedDecl*> DeclsInPrototypeScope;
+  /// Nonzero if we are currently parsing a function declarator. This is a counter
+  /// as opposed to a boolean so we can deal with nested function declarators
+  /// such as:
+  ///     void f(void (*g)(), ...)
+  unsigned InFunctionDeclarator;
+
   DeclGroupPtrTy ConvertDeclToDeclGroup(Decl *Ptr, Decl *OwnedType = 0);
 
   void DiagnoseUseOfUnimplementedSelectors();
@@ -1048,6 +1059,8 @@ public:
   // Returns true if the variable declaration is a redeclaration
   bool CheckVariableDeclaration(VarDecl *NewVD, LookupResult &Previous);
   void CheckCompleteVariableDeclaration(VarDecl *var);
+  void ActOnStartFunctionDeclarator();
+  void ActOnEndFunctionDeclarator();
   NamedDecl* ActOnFunctionDeclarator(Scope* S, Declarator& D, DeclContext* DC,
                                      TypeSourceInfo *TInfo,
                                      LookupResult &Previous,
diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp
index 433ce100cf..bd7579996f 100644
--- a/lib/AST/Decl.cpp
+++ b/lib/AST/Decl.cpp
@@ -1847,6 +1847,16 @@ void FunctionDecl::setParams(ASTContext &C,
   }
 }
 
+void FunctionDecl::setDeclsInPrototypeScope(llvm::ArrayRef<NamedDecl *> NewDecls) {
+  assert(DeclsInPrototypeScope.empty() && "Already has prototype decls!");
+
+  if (!NewDecls.empty()) {
+    NamedDecl **A = new (getASTContext()) NamedDecl*[NewDecls.size()];
+    std::copy(NewDecls.begin(), NewDecls.end(), A);
+    DeclsInPrototypeScope = llvm::ArrayRef<NamedDecl*>(A, NewDecls.size());
+  }
+}
+
 /// getMinRequiredArguments - Returns the minimum number of arguments
 /// needed to call this function. This may be fewer than the number of
 /// function parameters, if some of the parameters have default
diff --git a/lib/AST/DumpXML.cpp b/lib/AST/DumpXML.cpp
index b4038ddaf9..b180e808ab 100644
--- a/lib/AST/DumpXML.cpp
+++ b/lib/AST/DumpXML.cpp
@@ -497,6 +497,10 @@ struct XMLDumper : public XMLDeclVisitor<XMLDumper>,
     for (FunctionDecl::param_iterator
            I = D->param_begin(), E = D->param_end(); I != E; ++I)
       dispatch(*I);
+    for (llvm::ArrayRef<NamedDecl*>::iterator
+           I = D->getDeclsInPrototypeScope().begin(), E = D->getDeclsInPrototypeScope().end();
+         I != E; ++I)
+      dispatch(*I);
     if (D->doesThisDeclarationHaveABody())
       dispatch(D->getBody());
   }
diff --git a/lib/Parse/ParseDecl.cpp b/lib/Parse/ParseDecl.cpp
index 4d93eaf85d..2d8271db40 100644
--- a/lib/Parse/ParseDecl.cpp
+++ b/lib/Parse/ParseDecl.cpp
@@ -4226,6 +4226,8 @@ void Parser::ParseFunctionDeclarator(Declarator &D,
   ExprResult NoexceptExpr;
   ParsedType TrailingReturnType;
   
+  Actions.ActOnStartFunctionDeclarator();
+
   SourceLocation EndLoc;
   if (isFunctionDeclaratorIdentifierList()) {
     if (RequiresArg)
@@ -4308,6 +4310,8 @@ void Parser::ParseFunctionDeclarator(Declarator &D,
                                              EndLoc, D,
                                              TrailingReturnType),
                 attrs, EndLoc);
+
+  Actions.ActOnEndFunctionDeclarator();
 }
 
 /// isFunctionDeclaratorIdentifierList - This parameter list may have an
diff --git a/lib/Sema/IdentifierResolver.cpp b/lib/Sema/IdentifierResolver.cpp
index b4691617d0..218323134a 100644
--- a/lib/Sema/IdentifierResolver.cpp
+++ b/lib/Sema/IdentifierResolver.cpp
@@ -113,7 +113,7 @@ bool IdentifierResolver::isDeclInScope(Decl *D, DeclContext *Ctx,
                              bool ExplicitInstantiationOrSpecialization) const {
   Ctx = Ctx->getRedeclContext();
 
-  if (Ctx->isFunctionOrMethod()) {
+  if (Ctx->isFunctionOrMethod() || S->isFunctionPrototypeScope()) {
     // Ignore the scopes associated within transparent declaration contexts.
     while (S->getEntity() &&
            ((DeclContext *)S->getEntity())->isTransparentContext())
diff --git a/lib/Sema/Scope.cpp b/lib/Sema/Scope.cpp
index c76f61af6c..10f12ce844 100644
--- a/lib/Sema/Scope.cpp
+++ b/lib/Sema/Scope.cpp
@@ -59,3 +59,13 @@ void Scope::Init(Scope *parent, unsigned flags) {
   Entity = 0;
   ErrorTrap.reset();
 }
+
+bool Scope::containedInPrototypeScope() const {
+  const Scope *S = this;
+  while (S) {
+    if (S->isFunctionPrototypeScope())
+      return true;
+    S = S->getParent();
+  }
+  return false;
+}
diff --git a/lib/Sema/Sema.cpp b/lib/Sema/Sema.cpp
index cfb4e4c91b..bbb7af8aef 100644
--- a/lib/Sema/Sema.cpp
+++ b/lib/Sema/Sema.cpp
@@ -94,7 +94,7 @@ Sema::Sema(Preprocessor &pp, ASTContext &ctxt, ASTConsumer &consumer,
     ObjCShouldCallSuperDealloc(false),
     ObjCShouldCallSuperFinalize(false),
     TUKind(TUKind),
-    NumSFINAEErrors(0), SuppressAccessChecking(false), 
+    NumSFINAEErrors(0), InFunctionDeclarator(0), SuppressAccessChecking(false), 
     AccessCheckingSFINAE(false), InNonInstantiationSFINAEContext(false),
     NonInstantiationEntries(0), ArgumentPackSubstitutionIndex(-1),
     CurrentInstantiationScope(0), TyposCorrected(0),
diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp
index f091d858b5..7a0ae94739 100644
--- a/lib/Sema/SemaDecl.cpp
+++ b/lib/Sema/SemaDecl.cpp
@@ -1249,6 +1249,15 @@ void Sema::ActOnPopScope(SourceLocation Loc, Scope *S) {
   }
 }
 
+void Sema::ActOnStartFunctionDeclarator() {
+  ++InFunctionDeclarator;
+}
+
+void Sema::ActOnEndFunctionDeclarator() {
+  assert(InFunctionDeclarator);
+  --InFunctionDeclarator;
+}
+
 /// \brief Look for an Objective-C class in the translation unit.
 ///
 /// \param Id The name of the Objective-C class we're looking for. If
@@ -5241,6 +5250,15 @@ Sema::ActOnFunctionDeclarator(Scope *S, Declarator &D, DeclContext *DC,
   // Finally, we know we have the right number of parameters, install them.
   NewFD->setParams(Params);
 
+  // Find all anonymous symbols defined during the declaration of this function
+  // and add to NewFD. This lets us track decls such 'enum Y' in:
+  //
+  //   void f(enum Y {AA} x) {}
+  //
+  // which would otherwise incorrectly end up in the translation unit scope.
+  NewFD->setDeclsInPrototypeScope(DeclsInPrototypeScope);
+  DeclsInPrototypeScope.clear();
+
   // Process the non-inheritable attributes on this declaration.
   ProcessDeclAttributes(S, NewFD, D,
                         /*NonInheritable=*/true, /*Inheritable=*/false);
@@ -7225,6 +7243,43 @@ Decl *Sema::ActOnStartOfFunctionDef(Scope *FnBodyScope, Decl *D) {
     }
   }
 
+  // If we had any tags defined in the function prototype,
+  // introduce them into the function scope.
+  if (FnBodyScope) {
+    for (llvm::ArrayRef<NamedDecl*>::iterator I = FD->getDeclsInPrototypeScope().begin(),
+           E = FD->getDeclsInPrototypeScope().end(); I != E; ++I) {
+      NamedDecl *D = *I;
+
+      // Some of these decls (like enums) may have been pinned to the translation unit
+      // for lack of a real context earlier. If so, remove from the translation unit
+      // and reattach to the current context.
+      if (D->getLexicalDeclContext() == Context.getTranslationUnitDecl()) {
+        // Is the decl actually in the context?
+        for (DeclContext::decl_iterator DI = Context.getTranslationUnitDecl()->decls_begin(),
+               DE = Context.getTranslationUnitDecl()->decls_end(); DI != DE; ++DI) {
+          if (*DI == D) {  
+            Context.getTranslationUnitDecl()->removeDecl(D);
+            break;
+          }
+        }
+        // Either way, reassign the lexical decl context to our FunctionDecl.
+        D->setLexicalDeclContext(CurContext);
+      }
+
+      // If the decl has a non-null name, make accessible in the current scope.
+      if (!D->getName().empty())
+        PushOnScopeChains(D, FnBodyScope, /*AddToContext=*/false);
+
+      // Similarly, dive into enums and fish their constants out, making them
+      // accessible in this scope.
+      if (EnumDecl *ED = dyn_cast<EnumDecl>(D)) {
+        for (EnumDecl::enumerator_iterator EI = ED->enumerator_begin(),
+               EE = ED->enumerator_end(); EI != EE; ++EI)
+          PushOnScopeChains(*EI, FnBodyScope, /*AddToContext=*/false);
+      }
+    }
+  }
+
   // Checking attributes of current function definition
   // dllimport attribute.
   DLLImportAttr *DA = FD->getAttr<DLLImportAttr>();
@@ -8177,7 +8232,12 @@ Decl *Sema::ActOnTag(Scope *S, unsigned TagSpec, TagUseKind TUK,
                   !isa<CXXRecordDecl>(Def) ||
                   cast<CXXRecordDecl>(Def)->getTemplateSpecializationKind() 
                                                == TSK_ExplicitSpecialization) {
-                Diag(NameLoc, diag::err_redefinition) << Name;
+                // A redeclaration in function prototype scope in C isn't
+                // visible elsewhere, so merely issue a warning.
+                if (!getLangOptions().CPlusPlus && S->containedInPrototypeScope())
+                  Diag(NameLoc, diag::warn_redefinition_in_param_list) << Name;
+                else
+                  Diag(NameLoc, diag::err_redefinition) << Name;
                 Diag(Def->getLocation(), diag::note_previous_definition);
                 // If this is a redefinition, recover by making this
                 // struct be anonymous, which will make any later
@@ -8459,6 +8519,12 @@ CreateNewDecl:
         II->isStr("FILE"))
       Context.setFILEDecl(New);
 
+  // If we were in function prototype scope (and not in C++ mode), add this
+  // tag to the list of decls to inject into the function definition scope.
+  if (S->isFunctionPrototypeScope() && !getLangOptions().CPlusPlus &&
+      InFunctionDeclarator && Name)
+    DeclsInPrototypeScope.push_back(New);
+
   OwnedDecl = true;
   return New;
 }
@@ -10142,6 +10208,12 @@ void Sema::ActOnEnumBody(SourceLocation EnumLoc, SourceLocation LBraceLoc,
 
   Enum->completeDefinition(BestType, BestPromotionType,
                            NumPositiveBits, NumNegativeBits);
+
+  // If we're declaring a function, ensure this decl isn't forgotten about -
+  // it needs to go into the function scope.
+  if (InFunctionDeclarator)
+    DeclsInPrototypeScope.push_back(Enum);
+
 }
 
 Decl *Sema::ActOnFileScopeAsmDecl(Expr *expr,
diff --git a/test/CodeGen/decl-in-prototype.c b/test/CodeGen/decl-in-prototype.c
new file mode 100644
index 0000000000..949793da44
--- /dev/null
+++ b/test/CodeGen/decl-in-prototype.c
@@ -0,0 +1,21 @@
+// RUN: %clang -emit-llvm -S -o - %s | FileCheck %s
+
+const int AA = 5;
+
+// CHECK: define i32 @f1
+int f1(enum {AA,BB} E) {
+    // CHECK: ret i32 1
+    return BB;
+}
+
+// CHECK: define i32 @f2
+int f2(enum {AA=7,BB} E) {
+    // CHECK: ret i32 7
+    return AA;
+}
+
+// Check nested function declarators work.
+int f(void (*g)(), enum {AA,BB} h) {
+    // CHECK: ret i32 0
+    return AA;
+}
diff --git a/test/Misc/warning-flags.c b/test/Misc/warning-flags.c
index 6621f08fcf..5c0e80055a 100644
--- a/test/Misc/warning-flags.c
+++ b/test/Misc/warning-flags.c
@@ -17,7 +17,7 @@ This test serves two purposes:
 
 The list of warnings below should NEVER grow.  It should gradually shrink to 0.
 
-CHECK: Warnings without flags (257):
+CHECK: Warnings without flags (256):
 CHECK-NEXT:   ext_anonymous_struct_union_qualified
 CHECK-NEXT:   ext_binary_literal
 CHECK-NEXT:   ext_cast_fn_obj
@@ -137,7 +137,6 @@ CHECK-NEXT:   warn_conv_to_base_not_used
 CHECK-NEXT:   warn_conv_to_self_not_used
 CHECK-NEXT:   warn_conv_to_void_not_used
 CHECK-NEXT:   warn_cxx0x_right_shift_in_template_arg
-CHECK-NEXT:   warn_decl_in_param_list
 CHECK-NEXT:   warn_delete_array_type
 CHECK-NEXT:   warn_division_by_zero
 CHECK-NEXT:   warn_double_const_requires_fp64
diff --git a/test/Sema/decl-in-prototype.c b/test/Sema/decl-in-prototype.c
new file mode 100644
index 0000000000..05b8e0a1c3
--- /dev/null
+++ b/test/Sema/decl-in-prototype.c
@@ -0,0 +1,33 @@
+// RUN: %clang_cc1_only -verify %s
+
+const int AA = 5;
+
+int f1(enum {AA,BB} E) {
+    return BB;
+}
+
+int f2(enum {AA=7,BB} E) {
+    return AA;
+}
+
+struct a {
+};
+
+int f3(struct a { } *); // expected-warning {{will not be visible outside of this function}}
+
+struct A { struct b { int j; } t; }; // expected-note {{previous definition is here}}
+
+int f4(struct A { struct b { int j; } t; } *); // expected-warning {{declaration of 'struct A' will not be visible outside of this function}} expected-warning {{redefinition of 'b' will not be visible outside of this function}}
+
+struct aA {
+    struct ab { // expected-note {{previous definition is here}} expected-note {{previous definition is here}}
+        int j;
+    } b;
+};
+
+int f5(struct aA { struct ab { int j; } b; struct ab { char glorx; } glorx; } *); // expected-warning {{declaration of 'struct aA' will not be visible}} expected-warning {{redefinition of 'ab' will not be visible}} expected-warning {{redefinition of 'ab' will not be visible}}
+
+void f6(struct z {int b;} c) { // expected-warning {{declaration of 'struct z' will not be visible outside of this function}}
+    struct z d;
+    d.b = 4;
+}
diff --git a/test/Sema/fn-ptr-as-fn-prototype.c b/test/Sema/fn-ptr-as-fn-prototype.c
new file mode 100644
index 0000000000..cf95c97746
--- /dev/null
+++ b/test/Sema/fn-ptr-as-fn-prototype.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1_only -ast-print %s | FileCheck %s
+
+// This testcase checks the functionality of 
+// Sema::ActOn{Start,End}FunctionDeclarator, specifically checking that
+// ActOnEndFunctionDeclarator is called after the typedef so the enum
+// is in the global scope, not the scope of f().
+
+// CHECK: typedef void (*g)();
+typedef void (*g) ();
+// CHECK: enum {
+enum {
+  k = -1
+};
+// CHECK: void f() {
+void f() {}