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.

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

llvm-svn: 151712

clang/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

clang/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]>;

clang/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>;

clang/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;
 

clang/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,

clang/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

clang/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());
   }

clang/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

clang/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())

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

clang/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),

clang/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,

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

clang/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

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

clang/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() {}