Fix for merging decls in pragma weak

[amusman]

Summary

In the following example, we have a declaration of weakfoo before #pragma weak.

extern void weakfoo();
void localfoo() { }
#pragma weak weakfoo=localfoo

There are two decls for weakfoo, this leads to assertion failure later (during search a decl for a call to weakfoo):
lib/Sema/SemaOverload.cpp:5583: void clang::Sema::AddOverloadCandidate(clang::FunctionDecl*, clang::DeclAccessPair, llvm::ArrayRef<clang::Expr*>, clang::OverloadCandidateSet&, bool, bool, bool): Assertion `Proto && "Functions without a prototype cannot be overloaded"' failed.
I suggest to call CheckFunctionDeclaration so that 2 decls for the weakfoo are merged.

[aaron.ballman]

lib/Sema/SemaDeclAttr.cpp

5201 ↗

(On Diff #35348)

If we're in this code, any chance you can also look at the other FIXME statements? If not, that's fine -- this is still positive movement. But it's usually best to tackle all of these at once while it's fresh in someone's mind what's happening here.

[amusman]

lib/Sema/SemaDeclAttr.cpp

5201 ↗

(On Diff #35348)

I've got here through an existing test case that failed on assertion, not sure I can quickly write or find examples for the other fixme's... So, I'd prefer not to promise that :)

[aaron.ballman]

Generally LGTM, but I would wait for Richard to review in this case.

~Aaron

lib/Sema/SemaDeclAttr.cpp

5201 ↗

(On Diff #35348)

Ah, I hadn't noticed that this was Eli's code originally. I thought the original author was on the review thread for context. I wouldn't worry about it. If it's not problems in the last four years, we can live with it for a bit longer. ;-)

[aaron.ballman]

LGTM!

cfe/trunk/lib/Sema/SemaDeclAttr.cpp

 
 /// DeclClonePragmaWeak - clone existing decl (maybe definition),
 /// \#pragma weak needs a non-definition decl and source may not have one.
 NamedDecl * Sema::DeclClonePragmaWeak(NamedDecl *ND, IdentifierInfo *II,
                                       SourceLocation Loc) {
   assert(isa<FunctionDecl>(ND) || isa<VarDecl>(ND));
   NamedDecl *NewD = nullptr;
   if (FunctionDecl *FD = dyn_cast<FunctionDecl>(ND)) {
-    FunctionDecl *NewFD;
-    // FIXME: Missing call to CheckFunctionDeclaration().
     // FIXME: Mangling?
     // FIXME: Is the qualifier info correct?
     // FIXME: Is the DeclContext correct?
-    NewFD = FunctionDecl::Create(FD->getASTContext(), FD->getDeclContext(),
-                                 Loc, Loc, DeclarationName(II),
-                                 FD->getType(), FD->getTypeSourceInfo(),
-                                 SC_None, false/*isInlineSpecified*/,
-                                 FD->hasPrototype(),
-                                 false/*isConstexprSpecified*/);
+
+    LookupResult Previous(*this, II, Loc, LookupOrdinaryName);
+    LookupParsedName(Previous, TUScope, nullptr, true);
+
+    auto NewFD = FunctionDecl::Create(
+        FD->getASTContext(), FD->getDeclContext(), Loc, Loc,
+        DeclarationName(II), FD->getType(), FD->getTypeSourceInfo(), SC_None,
+        false /*isInlineSpecified*/, FD->hasPrototype(),
+        false /*isConstexprSpecified*/);
+
+    CheckFunctionDeclaration(TUScope, NewFD, Previous,
+                             false /*IsExplicitSpecialization*/);
+
     NewD = NewFD;
 
     if (FD->getQualifier())
       NewFD->setQualifierInfo(FD->getQualifierLoc());
 
     // Fake up parameter variables; they are declared as if this were
     // a typedef.
     QualType FDTy = FD->getType();

cfe/trunk/test/CodeGen/pragma-weak.c

 // CHECK-DAG: @foo2 = weak alias void (), void ()* @__foo2
 // CHECK-DAG: @stutter = weak alias void (), void ()* @__stutter
 // CHECK-DAG: @stutter2 = weak alias void (), void ()* @__stutter2
 // CHECK-DAG: @declfirst = weak alias void (), void ()* @__declfirst
 // CHECK-DAG: @declfirstattr = weak alias void (), void ()* @__declfirstattr
 // CHECK-DAG: @mix2 = weak alias void (), void ()* @__mix2
 // CHECK-DAG: @a1 = weak alias void (), void ()* @__a1
 // CHECK-DAG: @xxx = weak alias void (), void ()* @__xxx
+// CHECK-DAG: @weakfoo = weak alias void {{.*}} @localfoo
 
 
 
 // CHECK-LABEL: define weak void @weakdef()
 
 
 #pragma weak weakvar
 int weakvar;
label:
   #pragma weak PR16705c
   PR16705c();
 }
 
 // CHECK: declare extern_weak i32 @PR16705a()
 // CHECK: declare extern_weak i32 @PR16705b()
 // CHECK: declare extern_weak i32 @PR16705c()
 
+// In this test case, we have a declaration of weakfoo before #pragma weak.
+// Test that 2 decls for the weakfoo are merged.
+extern void weakfoo();
+void localfoo() { }
+#pragma weak weakfoo=localfoo
+extern void externmain() { return weakfoo(); }
+// CHECK-LABEL: define void @externmain()
+// CHECK: call{{.*}}@weakfoo
 
 ///////////// TODO: stuff that still doesn't work
 
 // due to the fact that disparate TopLevelDecls cannot affect each other
 // (due to clang's Parser and ASTConsumer behavior, and quite reasonable)
 // #pragma weak must appear before or within the same TopLevelDecl as it
 // references.
 void yyy(void){}

cfe/trunk/test/Sema/pragma-weak.c

 // RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -fsyntax-only -verify %s
 
 void __both3(void);
 #pragma weak both3 = __both3 // expected-note {{previous definition}}
 void both3(void) __attribute((alias("__both3"))); // expected-error {{redefinition of 'both3'}}
 void __both3(void) {}
 
 void __a3(void) __attribute((noinline));
 #pragma weak a3 = __a3 // expected-note {{previous definition}}
 void a3(void) __attribute((alias("__a3"))); // expected-error {{redefinition of 'a3'}}
 void __a3(void) {}
+
+extern void weak2foo(int); // expected-note {{previous declaration is here}} expected-note {{'weak2foo' declared here}}
+void local2foo(double d1, double d2) { }
+#pragma weak weak2foo=local2foo // expected-error {{conflicting types for 'weak2foo'}}
+extern void extern2main() { return weak2foo(); } // expected-error {{too few arguments to function call, expected 1, have 0}}
+