Adopt a more principled approach to invalid declarations:

  - If a declaration is an invalid redeclaration of an existing name,
    complain about the invalid redeclaration then avoid adding it to
    the AST (we can still parse the definition or initializer, if any).
  - If the declaration is invalid but there is no prior declaration
    with that name, introduce the invalid declaration into the AST
    (for later error recovery).
  - If the declaration is an invalid redeclaration of a builtin that
    starts with __builtin_, we produce an error and drop the
    redeclaration. If it is an invalid redeclaration of a library
    builtin (e.g., malloc, printf), warn (don't error!) and drop the
    redeclaration.

If a user attempts to define a builtin, produce an error and (if it's
a library builtin like malloc) suggest -ffreestanding.

This addresses <rdar://problem/6097585> and PR2892. However, PR3588 is
still going to cause some problems when builtins are redeclared
without a prototype.

llvm-svn: 64639

3 files changed, 101 insertions, 72 deletions

diff --git a/clang/lib/Sema/Sema.h b/clang/lib/Sema/Sema.h
index 362d8dc02de..a96618f5a93 100644
--- a/clang/lib/Sema/Sema.h
+++ b/clang/lib/Sema/Sema.h
@@ -282,14 +282,16 @@ public:
                           bool IsFunctionDefinition);
   NamedDecl* ActOnTypedefDeclarator(Scope* S, Declarator& D, DeclContext* DC,
                                     QualType R, Decl* LastDeclarator,
-                                    Decl* PrevDecl, bool& InvalidDecl);
+                                    Decl* PrevDecl, bool& InvalidDecl,
+                                    bool &Redeclaration);
   NamedDecl* ActOnVariableDeclarator(Scope* S, Declarator& D, DeclContext* DC,
-                                      QualType R, Decl* LastDeclarator,
-                                      Decl* PrevDecl, bool& InvalidDecl);
+                                     QualType R, Decl* LastDeclarator,
+                                     Decl* PrevDecl, bool& InvalidDecl,
+                                     bool &Redeclaration);
   NamedDecl* ActOnFunctionDeclarator(Scope* S, Declarator& D, DeclContext* DC,
                                      QualType R, Decl *LastDeclarator,
                                      Decl* PrevDecl, bool IsFunctionDefinition,
-                                     bool& InvalidDecl);
+                                     bool& InvalidDecl, bool &Redeclaration);
   virtual DeclTy *ActOnParamDeclarator(Scope *S, Declarator &D);
   virtual void ActOnParamDefaultArgument(DeclTy *param, 
                                          SourceLocation EqualLoc,
@@ -395,11 +397,10 @@ public:
   /// Subroutines of ActOnDeclarator().
   TypedefDecl *ParseTypedefDecl(Scope *S, Declarator &D, QualType T,
                                 Decl *LastDecl);
-  TypedefDecl *MergeTypeDefDecl(TypedefDecl *New, Decl *Old);
-  FunctionDecl *MergeFunctionDecl(FunctionDecl *New, Decl *Old, 
-                                  bool &Redeclaration);
-  VarDecl *MergeVarDecl(VarDecl *New, Decl *Old);
-  FunctionDecl *MergeCXXFunctionDecl(FunctionDecl *New, FunctionDecl *Old);
+  bool MergeTypeDefDecl(TypedefDecl *New, Decl *Old);
+  bool MergeFunctionDecl(FunctionDecl *New, Decl *Old);
+  bool MergeVarDecl(VarDecl *New, Decl *Old);
+  bool MergeCXXFunctionDecl(FunctionDecl *New, FunctionDecl *Old);
   void CheckForFileScopedRedefinitions(Scope *S, VarDecl *VD);
 
   /// C++ Overloading.
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index a47094ecfed..3960a7d57c0 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -364,11 +364,13 @@ NamespaceDecl *Sema::GetStdNamespace() {
   return StdNamespace;
 }
 
-/// MergeTypeDefDecl - We just parsed a typedef 'New' which has the same name
-/// and scope as a previous declaration 'Old'.  Figure out how to resolve this
-/// situation, merging decls or emitting diagnostics as appropriate.
+/// MergeTypeDefDecl - We just parsed a typedef 'New' which has the
+/// same name and scope as a previous declaration 'Old'.  Figure out
+/// how to resolve this situation, merging decls or emitting
+/// diagnostics as appropriate. Returns true if there was an error,
+/// false otherwise.
 ///
-TypedefDecl *Sema::MergeTypeDefDecl(TypedefDecl *New, Decl *OldD) {
+bool Sema::MergeTypeDefDecl(TypedefDecl *New, Decl *OldD) {
   bool objc_types = false;
   // Allow multiple definitions for ObjC built-in typedefs.
   // FIXME: Verify the underlying types are equivalent!
@@ -387,19 +389,19 @@ TypedefDecl *Sema::MergeTypeDefDecl(TypedefDecl *New, Decl *OldD) {
         break;
       Context.setObjCClassType(New);
       objc_types = true;
-      return New;
+      return false;
     case 3:
       if (!TypeID->isStr("SEL"))
         break;
       Context.setObjCSelType(New);
       objc_types = true;
-      return New;
+      return false;
     case 8:
       if (!TypeID->isStr("Protocol"))
         break;
       Context.setObjCProtoType(New->getUnderlyingType());
       objc_types = true;
-      return New;
+      return false;
     }
     // Fall through - the typedef name was not a builtin type.
   }
@@ -410,7 +412,7 @@ TypedefDecl *Sema::MergeTypeDefDecl(TypedefDecl *New, Decl *OldD) {
       << New->getDeclName();
     if (!objc_types)
       Diag(OldD->getLocation(), diag::note_previous_definition);
-    return New;
+    return true;
   }
 
   // Determine the "old" type we'll use for checking and diagnostics.  
@@ -430,17 +432,17 @@ TypedefDecl *Sema::MergeTypeDefDecl(TypedefDecl *New, Decl *OldD) {
       << New->getUnderlyingType() << OldType;
     if (!objc_types)
       Diag(Old->getLocation(), diag::note_previous_definition);
-    return New;
+    return true;
   }
-  if (objc_types) return New;
-  if (getLangOptions().Microsoft) return New;
+  if (objc_types) return false;
+  if (getLangOptions().Microsoft) return false;
 
   // C++ [dcl.typedef]p2:
   //   In a given non-class scope, a typedef specifier can be used to
   //   redefine the name of any type declared in that scope to refer
   //   to the type to which it already refers.
   if (getLangOptions().CPlusPlus && !isa<CXXRecordDecl>(CurContext))
-    return New;
+    return false;
 
   // In C, redeclaration of a type is a constraint violation (6.7.2.3p1).
   // Apparently GCC, Intel, and Sun all silently ignore the redeclaration if
@@ -450,14 +452,14 @@ TypedefDecl *Sema::MergeTypeDefDecl(TypedefDecl *New, Decl *OldD) {
   if (PP.getDiagnostics().getSuppressSystemWarnings()) {
     SourceManager &SrcMgr = Context.getSourceManager();
     if (SrcMgr.isInSystemHeader(Old->getLocation()))
-      return New;
+      return false;
     if (SrcMgr.isInSystemHeader(New->getLocation()))
-      return New;
+      return false;
   }
 
   Diag(New->getLocation(), diag::err_redefinition) << New->getDeclName();
   Diag(Old->getLocation(), diag::note_previous_definition);
-  return New;
+  return true;
 }
 
 /// DeclhasAttr - returns true if decl Declaration already has the target
@@ -494,25 +496,24 @@ static void MergeAttributes(Decl *New, Decl *Old) {
 /// declarator D which has the same name and scope as a previous
 /// declaration 'Old'.  Figure out how to resolve this situation,
 /// merging decls or emitting diagnostics as appropriate.
-/// Redeclaration will be set true if this New is a redeclaration OldD.
 ///
 /// In C++, New and Old must be declarations that are not
 /// overloaded. Use IsOverload to determine whether New and Old are
 /// overloaded, and to select the Old declaration that New should be
 /// merged with.
-FunctionDecl *
-Sema::MergeFunctionDecl(FunctionDecl *New, Decl *OldD, bool &Redeclaration) {
+///
+/// Returns true if there was an error, false otherwise.
+bool Sema::MergeFunctionDecl(FunctionDecl *New, Decl *OldD) {
   assert(!isa<OverloadedFunctionDecl>(OldD) && 
          "Cannot merge with an overloaded function declaration");
 
-  Redeclaration = false;
   // Verify the old decl was also a function.
   FunctionDecl *Old = dyn_cast<FunctionDecl>(OldD);
   if (!Old) {
     Diag(New->getLocation(), diag::err_redefinition_different_kind)
       << New->getDeclName();
     Diag(OldD->getLocation(), diag::note_previous_definition);
-    return New;
+    return true;
   }
 
   // Determine whether the previous declaration was a definition,
@@ -520,12 +521,9 @@ Sema::MergeFunctionDecl(FunctionDecl *New, Decl *OldD, bool &Redeclaration) {
   diag::kind PrevDiag;
   if (Old->isThisDeclarationADefinition())
     PrevDiag = diag::note_previous_definition;
-  else if (Old->isImplicit()) {
-    if (Old->getBuiltinID(Context))
-      PrevDiag = diag::note_previous_builtin_declaration;
-    else
-      PrevDiag = diag::note_previous_implicit_declaration;
-  } else 
+  else if (Old->isImplicit())
+    PrevDiag = diag::note_previous_implicit_declaration;
+  else 
     PrevDiag = diag::note_previous_declaration;
   
   QualType OldQType = Context.getCanonicalType(Old->getType());
@@ -543,8 +541,7 @@ Sema::MergeFunctionDecl(FunctionDecl *New, Decl *OldD, bool &Redeclaration) {
     if (OldReturnType != NewReturnType) {
       Diag(New->getLocation(), diag::err_ovl_diff_return_type);
       Diag(Old->getLocation(), PrevDiag) << Old << Old->getType();
-      Redeclaration = true;
-      return New;
+      return true;
     }
 
     const CXXMethodDecl* OldMethod = dyn_cast<CXXMethodDecl>(Old);
@@ -556,7 +553,7 @@ Sema::MergeFunctionDecl(FunctionDecl *New, Decl *OldD, bool &Redeclaration) {
       if (OldMethod->isStatic() || NewMethod->isStatic()) {
         Diag(New->getLocation(), diag::err_ovl_static_nonstatic_member);
         Diag(Old->getLocation(), PrevDiag) << Old << Old->getType();
-        return New;
+        return true;
       }
 
       // C++ [class.mem]p1:
@@ -586,7 +583,6 @@ Sema::MergeFunctionDecl(FunctionDecl *New, Decl *OldD, bool &Redeclaration) {
     if (OldQType == NewQType) {
       // We have a redeclaration.
       MergeAttributes(New, Old);
-      Redeclaration = true;
       return MergeCXXFunctionDecl(New, Old);
     } 
 
@@ -598,19 +594,30 @@ Sema::MergeFunctionDecl(FunctionDecl *New, Decl *OldD, bool &Redeclaration) {
   if (!getLangOptions().CPlusPlus &&
       Context.typesAreCompatible(OldQType, NewQType)) {
     MergeAttributes(New, Old);
-    Redeclaration = true;
-    return New;
+    return false;
   }
 
   // A function that has already been declared has been redeclared or defined
   // with a different type- show appropriate diagnostic
+  if (unsigned BuiltinID = Old->getBuiltinID(Context)) {
+    // The user has declared a builtin function with an incompatible
+    // signature.
+    if (Context.BuiltinInfo.isPredefinedLibFunction(BuiltinID)) {
+      // The function the user is redeclaring is a library-defined
+      // function like 'malloc' or 'printf'. Warn about the
+      // redeclaration, then ignore it.
+      Diag(New->getLocation(), diag::warn_redecl_library_builtin) << New;
+      Diag(Old->getLocation(), diag::note_previous_builtin_declaration)
+        << Old << Old->getType();
+      return false;
+    }
+
+    PrevDiag = diag::note_previous_builtin_declaration;
+  }
 
-  // TODO: CHECK FOR CONFLICTS, multiple decls with same name in one scope.
-  // TODO: This is totally simplistic.  It should handle merging functions
-  // together etc, merging extern int X; int X; ...
   Diag(New->getLocation(), diag::err_conflicting_types) << New->getDeclName();
   Diag(Old->getLocation(), PrevDiag) << Old << Old->getType();
-  return New;
+  return true;
 }
 
 /// Predicate for C "tentative" external object definitions (C99 6.9.2).
@@ -671,14 +678,14 @@ void Sema::CheckForFileScopedRedefinitions(Scope *S, VarDecl *VD) {
 /// FinalizeDeclaratorGroup. Unfortunately, we can't analyze tentative 
 /// definitions here, since the initializer hasn't been attached.
 /// 
-VarDecl *Sema::MergeVarDecl(VarDecl *New, Decl *OldD) {
+bool Sema::MergeVarDecl(VarDecl *New, Decl *OldD) {
   // Verify the old decl was also a variable.
   VarDecl *Old = dyn_cast<VarDecl>(OldD);
   if (!Old) {
     Diag(New->getLocation(), diag::err_redefinition_different_kind)
       << New->getDeclName();
     Diag(OldD->getLocation(), diag::note_previous_definition);
-    return New;
+    return true;
   }
 
   MergeAttributes(New, Old);
@@ -689,7 +696,7 @@ VarDecl *Sema::MergeVarDecl(VarDecl *New, Decl *OldD) {
     Diag(New->getLocation(), diag::err_redefinition_different_type) 
       << New->getDeclName();
     Diag(Old->getLocation(), diag::note_previous_definition);
-    return New;
+    return true;
   }
   New->setType(MergedT);
   // C99 6.2.2p4: Check if we have a static decl followed by a non-static.
@@ -698,21 +705,22 @@ VarDecl *Sema::MergeVarDecl(VarDecl *New, Decl *OldD) {
        Old->getStorageClass() == VarDecl::Extern)) {
     Diag(New->getLocation(), diag::err_static_non_static) << New->getDeclName();
     Diag(Old->getLocation(), diag::note_previous_definition);
-    return New;
+    return true;
   }
   // C99 6.2.2p4: Check if we have a non-static decl followed by a static.
   if (New->getStorageClass() != VarDecl::Static &&
       Old->getStorageClass() == VarDecl::Static) {
     Diag(New->getLocation(), diag::err_non_static_static) << New->getDeclName();
     Diag(Old->getLocation(), diag::note_previous_definition);
-    return New;
+    return true;
   }
   // Variables with external linkage are analyzed in FinalizeDeclaratorGroup.
   if (New->getStorageClass() != VarDecl::Extern && !New->isFileVarDecl()) {
     Diag(New->getLocation(), diag::err_redefinition) << New->getDeclName();
     Diag(Old->getLocation(), diag::note_previous_definition);
+    return true;
   }
-  return New;
+  return false;
 }
 
 /// CheckParmsForFunctionDef - Check that the parameters of the given
@@ -1245,7 +1253,7 @@ Sema::ActOnDeclarator(Scope *S, Declarator &D, DeclTy *lastDecl,
   NamedDecl *PrevDecl;
   NamedDecl *New;
   bool InvalidDecl = false;
- 
+
   // See if this is a redefinition of a variable in the same scope.
   if (!D.getCXXScopeSpec().isSet() && !D.getCXXScopeSpec().isInvalid()) {
     DC = CurContext;
@@ -1315,15 +1323,17 @@ Sema::ActOnDeclarator(Scope *S, Declarator &D, DeclTy *lastDecl,
   QualType R = GetTypeForDeclarator(D, S);
   assert(!R.isNull() && "GetTypeForDeclarator() returned null type");
 
+  bool Redeclaration = false;
   if (D.getDeclSpec().getStorageClassSpec() == DeclSpec::SCS_typedef) {
     New = ActOnTypedefDeclarator(S, D, DC, R, LastDeclarator, PrevDecl,
-                                 InvalidDecl);
+                                 InvalidDecl, Redeclaration);
   } else if (R.getTypePtr()->isFunctionType()) {
     New = ActOnFunctionDeclarator(S, D, DC, R, LastDeclarator, PrevDecl, 
-                                  IsFunctionDefinition, InvalidDecl);
+                                  IsFunctionDefinition, InvalidDecl,
+                                  Redeclaration);
   } else {
     New = ActOnVariableDeclarator(S, D, DC, R, LastDeclarator, PrevDecl, 
-                                  InvalidDecl);
+                                  InvalidDecl, Redeclaration);
   }
 
   if (New == 0)
@@ -1333,8 +1343,9 @@ Sema::ActOnDeclarator(Scope *S, Declarator &D, DeclTy *lastDecl,
   // lexical context will be different from the semantic context.
   New->setLexicalDeclContext(CurContext);
 
-  // If this has an identifier, add it to the scope stack.
-  if (Name)
+  // If this has an identifier and is not an invalid redeclaration,
+  // add it to the scope stack.
+  if (Name && !(Redeclaration && InvalidDecl))
     PushOnScopeChains(New, S);
   // If any semantic error occurred, mark the decl as invalid.
   if (D.getInvalidType() || InvalidDecl)
@@ -1346,7 +1357,8 @@ Sema::ActOnDeclarator(Scope *S, Declarator &D, DeclTy *lastDecl,
 NamedDecl*
 Sema::ActOnTypedefDeclarator(Scope* S, Declarator& D, DeclContext* DC,
                              QualType R, Decl* LastDeclarator,
-                             Decl* PrevDecl, bool& InvalidDecl) {
+                             Decl* PrevDecl, bool& InvalidDecl, 
+                             bool &Redeclaration) {
   // Typedef declarators cannot be qualified (C++ [dcl.meaning]p1).
   if (D.getCXXScopeSpec().isSet()) {
     Diag(D.getIdentifierLoc(), diag::err_qualified_typedef_declarator)
@@ -1368,8 +1380,9 @@ Sema::ActOnTypedefDeclarator(Scope* S, Declarator& D, DeclContext* DC,
   // Merge the decl with the existing one if appropriate. If the decl is
   // in an outer scope, it isn't the same thing.
   if (PrevDecl && isDeclInScope(PrevDecl, DC, S)) {
-    NewTD = MergeTypeDefDecl(NewTD, PrevDecl);
-    if (NewTD == 0) return 0;
+    Redeclaration = true;
+    if (MergeTypeDefDecl(NewTD, PrevDecl))
+      InvalidDecl = true;
   }
 
   if (S->getFnParent() == 0) {
@@ -1390,7 +1403,8 @@ Sema::ActOnTypedefDeclarator(Scope* S, Declarator& D, DeclContext* DC,
 NamedDecl*
 Sema::ActOnVariableDeclarator(Scope* S, Declarator& D, DeclContext* DC,
                               QualType R, Decl* LastDeclarator,
-                              Decl* PrevDecl, bool& InvalidDecl) {
+                              Decl* PrevDecl, bool& InvalidDecl,
+                              bool &Redeclaration) {
   DeclarationName Name = GetNameForDeclarator(D);
 
   // Check that there are no default arguments (C++ only).
@@ -1483,8 +1497,9 @@ Sema::ActOnVariableDeclarator(Scope* S, Declarator& D, DeclContext* DC,
       return 0;
     }
 
-    NewVD = MergeVarDecl(NewVD, PrevDecl);
-    if (NewVD == 0) return 0;
+    Redeclaration = true;
+    if (MergeVarDecl(NewVD, PrevDecl))
+      InvalidDecl = true;
 
     if (D.getCXXScopeSpec().isSet()) {
       // No previous declaration in the qualifying scope.
@@ -1500,7 +1515,7 @@ NamedDecl*
 Sema::ActOnFunctionDeclarator(Scope* S, Declarator& D, DeclContext* DC,
                               QualType R, Decl *LastDeclarator,
                               Decl* PrevDecl, bool IsFunctionDefinition,
-                              bool& InvalidDecl) {
+                              bool& InvalidDecl, bool &Redeclaration) {
   assert(R.getTypePtr()->isFunctionType());
 
   DeclarationName Name = GetNameForDeclarator(D);
@@ -1691,7 +1706,6 @@ Sema::ActOnFunctionDeclarator(Scope* S, Declarator& D, DeclContext* DC,
   // Merge the decl with the existing one if appropriate. Since C functions
   // are in a flat namespace, make sure we consider decls in outer scopes.
   bool OverloadableAttrRequired = false;
-  bool Redeclaration = false;
   if (PrevDecl &&
       (!getLangOptions().CPlusPlus||isDeclInScope(PrevDecl, DC, S))) {
     // Determine whether NewFD is an overload of PrevDecl or
@@ -1706,6 +1720,7 @@ Sema::ActOnFunctionDeclarator(Scope* S, Declarator& D, DeclContext* DC,
 
     if (!AllowOverloadingOfFunction(PrevDecl, Context) || 
         !IsOverload(NewFD, PrevDecl, MatchedDecl)) {
+      Redeclaration = true;
       Decl *OldDecl = PrevDecl;
 
       // If PrevDecl was an overloaded function, extract the
@@ -1715,10 +1730,10 @@ Sema::ActOnFunctionDeclarator(Scope* S, Declarator& D, DeclContext* DC,
 
       // NewFD and PrevDecl represent declarations that need to be
       // merged. 
-      NewFD = MergeFunctionDecl(NewFD, OldDecl, Redeclaration);
+      if (MergeFunctionDecl(NewFD, OldDecl))
+        InvalidDecl = true;
 
-      if (NewFD == 0) return 0;
-      if (Redeclaration) {
+      if (!InvalidDecl) {
         NewFD->setPreviousDeclaration(cast<FunctionDecl>(OldDecl));
 
         // An out-of-line member function declaration must also be a
@@ -2734,6 +2749,16 @@ Sema::DeclTy *Sema::ActOnStartOfFunctionDef(Scope *FnBodyScope, DeclTy *D) {
     Diag(Definition->getLocation(), diag::note_previous_definition);
   }
 
+  // Builtin functions cannot be defined.
+  if (unsigned BuiltinID = FD->getBuiltinID(Context)) {
+    if (Context.BuiltinInfo.isPredefinedLibFunction(BuiltinID)) {
+      Diag(FD->getLocation(), diag::err_builtin_lib_definition) << FD;
+      Diag(FD->getLocation(), diag::note_builtin_lib_def_freestanding);
+    } else 
+      Diag(FD->getLocation(), diag::err_builtin_definition) << FD;
+    FD->setInvalidDecl();
+  }
+
   PushDeclContext(FnBodyScope, FD);
 
   // Check the validity of our function parameters
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 02c2e9bbfab..4af8cc030b9 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -201,9 +201,11 @@ void Sema::CheckExtraCXXDefaultArguments(Declarator &D) {
 
 // MergeCXXFunctionDecl - Merge two declarations of the same C++
 // function, once we already know that they have the same
-// type. Subroutine of MergeFunctionDecl.
-FunctionDecl * 
-Sema::MergeCXXFunctionDecl(FunctionDecl *New, FunctionDecl *Old) {
+// type. Subroutine of MergeFunctionDecl. Returns true if there was an
+// error, false otherwise.
+bool Sema::MergeCXXFunctionDecl(FunctionDecl *New, FunctionDecl *Old) {
+  bool Invalid = false;
+
   // C++ [dcl.fct.default]p4:
   //
   //   For non-template functions, default arguments can be added in
@@ -226,13 +228,14 @@ Sema::MergeCXXFunctionDecl(FunctionDecl *New, FunctionDecl *Old) {
            diag::err_param_default_argument_redefinition)
         << NewParam->getDefaultArg()->getSourceRange();
       Diag(OldParam->getLocation(), diag::note_previous_definition);
+      Invalid = true;
     } else if (OldParam->getDefaultArg()) {
       // Merge the old default argument into the new parameter
       NewParam->setDefaultArg(OldParam->getDefaultArg());
     }
   }
 
-  return New;  
+  return Invalid;
 }
 
 /// CheckCXXDefaultArguments - Verify that the default arguments for a