Fix: Prevent regular definitions from being weakened to available_externally

This is exactly what happened for the 4 problematic delegate literals
for the Phobos unittests - DtoDefineFunction(fd) could result in that
definition ending up as available_externally - all done as part of
DtoResolveFunction(), which declared it, and DtoDeclareFunction()
defined it as available_externally, and the outer DtoDefineFunction()
returned early in that case without fixing up the linkage.

gen/functions.cpp

@@ -345,7 +345,7 @@ static llvm::Function *DtoDeclareVaFunction(FuncDeclaration *fdecl) {
 
 ////////////////////////////////////////////////////////////////////////////////
 
-void DtoResolveFunction(FuncDeclaration *fdecl) {
+void DtoResolveFunction(FuncDeclaration *fdecl, const bool willDeclare) {
   if ((!global.params.useUnitTests || !fdecl->type) &&
       fdecl->isUnitTestDeclaration()) {
     IF_LOG Logger::println("Ignoring unittest %s", fdecl->toPrettyChars());
@@ -423,11 +423,15 @@ void DtoResolveFunction(FuncDeclaration *fdecl) {
   LOG_SCOPE;
 
   // queue declaration unless the function is abstract without body
-  if (!fdecl->isAbstract() || fdecl->fbody) {
+  if (!willDeclare && (!fdecl->isAbstract() || fdecl->fbody)) {
     DtoDeclareFunction(fdecl);
   }
 }
 
+void DtoResolveFunction(FuncDeclaration *fdecl) {
+  return DtoResolveFunction(fdecl, false);
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 
 namespace {
@@ -539,8 +543,8 @@ void onlyOneMainCheck(FuncDeclaration *fd) {
 
 ////////////////////////////////////////////////////////////////////////////////
 
-void DtoDeclareFunction(FuncDeclaration *fdecl) {
-  DtoResolveFunction(fdecl);
+void DtoDeclareFunction(FuncDeclaration *fdecl, const bool willDefine) {
+  DtoResolveFunction(fdecl, /*willDeclare=*/true);
 
   if (fdecl->ir->isDeclared()) {
     return;
@@ -567,7 +571,7 @@ void DtoDeclareFunction(FuncDeclaration *fdecl) {
   // Check if fdecl should be defined too for cross-module inlining.
   // If true, semantic is fully done for fdecl which is needed for some code
   // below (e.g. code that uses fdecl->vthis).
-  const bool defineAtEnd = defineAsExternallyAvailable(*fdecl);
+  const bool defineAtEnd = !willDefine && defineAsExternallyAvailable(*fdecl);
   if (defineAtEnd) {
     IF_LOG Logger::println(
         "Function is an externally_available inline candidate.");
@@ -762,10 +766,14 @@ void DtoDeclareFunction(FuncDeclaration *fdecl) {
   if (defineAtEnd) {
     IF_LOG Logger::println(
         "Function is an externally_available inline candidate: define it now.");
-    DtoDefineFunction(fdecl, true);
+    DtoDefineFunction(fdecl, /*linkageAvailableExternally=*/true);
   }
 }
 
+void DtoDeclareFunction(FuncDeclaration *fdecl) {
+  return DtoDeclareFunction(fdecl, false);
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 
 static LinkageWithCOMDAT lowerFuncLinkage(FuncDeclaration *fdecl) {
@@ -1001,12 +1009,19 @@ void DtoDefineFunction(FuncDeclaration *fd, bool linkageAvailableExternally) {
     fatal();
   }
 
-  DtoResolveFunction(fd);
+  DtoDeclareFunction(fd, /*willDefine=*/true);
+  assert(fd->ir->isDeclared());
+
+  // DtoDeclareFunction might also set the defined flag for functions we
+  // should not touch.
+  if (fd->ir->isDefined()) {
+    return;
+  }
+  fd->ir->setDefined();
 
   if (fd->isUnitTestDeclaration() && !global.params.useUnitTests) {
     IF_LOG Logger::println("No code generation for unit test declaration %s",
                            fd->toChars());
-    fd->ir->setDefined();
     return;
   }
 
@@ -1016,27 +1031,15 @@ void DtoDefineFunction(FuncDeclaration *fd, bool linkageAvailableExternally) {
       IF_LOG Logger::println(
           "No code generation for typeinfo member %s in @compute code",
           fd->toChars());
-      fd->ir->setDefined();
       return;
     }
   }
 
   if (!linkageAvailableExternally && !alreadyOrWillBeDefined(*fd)) {
     IF_LOG Logger::println("Skipping '%s'.", fd->toPrettyChars());
-    fd->ir->setDefined();
     return;
   }
 
-  DtoDeclareFunction(fd);
-  assert(fd->ir->isDeclared());
-
-  // DtoResolveFunction might also set the defined flag for functions we
-  // should not touch.
-  if (fd->ir->isDefined()) {
-    return;
-  }
-  fd->ir->setDefined();
-
   // We cannot emit nested functions with parents that have not gone through
   // semantic analysis. This can happen as DMD leaks some template instances
   // from constraints into the module member list. DMD gets away with being

gen/tollvm.cpp

@@ -215,8 +215,11 @@ LLValue *DtoDelegateEquals(TOK op, LLValue *lhs, LLValue *rhs) {
 ////////////////////////////////////////////////////////////////////////////////
 
 LinkageWithCOMDAT DtoLinkage(Dsymbol *sym) {
-  auto linkage = (DtoIsTemplateInstance(sym) ? templateLinkage
-                                             : LLGlobalValue::ExternalLinkage);
+  // Function (incl. delegate) literals are emitted into each referencing
+  // compilation unit; use template linkage to prevent conflicts.
+  auto linkage = (sym->isFuncLiteralDeclaration() || DtoIsTemplateInstance(sym))
+                     ? templateLinkage
+                     : LLGlobalValue::ExternalLinkage;
 
   // If @(ldc.attributes.weak) is applied, override the linkage to WeakAny
   if (hasWeakUDA(sym)) {

tests/codegen/inlining_gh3126.d

@@ -1,14 +1,20 @@
 // Tests that functions are cross-module inlined when emitting multiple object
 // files.
 
-// Generate unoptimized IR for 2 source files as separate compilation units.
-// RUN: %ldc -c -output-ll %s %S/inputs/inlinables.d -od=%t
-// RUN: FileCheck %s < %t/inlining_gh3126.ll
+// Generate unoptimized IR for 2 source files as separate compilation units (in both orders).
+// RUN: %ldc -c -output-ll %s %S/inputs/inlinables.d -od=%t && FileCheck %s < %t/inlining_gh3126.ll
+// RUN: %ldc -c -output-ll %S/inputs/inlinables.d %s -od=%t && FileCheck %s < %t/inlining_gh3126.ll
+
+// Now test with another source file making use of inputs.inlinables instead of compiling that module directly.
+// RUN: %ldc -c -output-ll -I%S %s %S/inlining_imports_pragma.d -od=%t && FileCheck %s < %t/inlining_gh3126.ll
+// RUN: %ldc -c -output-ll -I%S %S/inlining_imports_pragma.d %s -od=%t && FileCheck %s < %t/inlining_gh3126.ll
 
 import inputs.inlinables;
 
-// no other function definitions (always_inline_chain*)
-// CHECK-NOT: define
+// no other function definitions (always_inline_chain*, call_template_foo);
+// allow template_foo to be instantiated in here though
+// CHECK-NOT: always_inline_chain
+// CHECK-NOT: call_template_foo
 
 // CHECK: define {{.*}}_D15inlining_gh31263fooFZi
 int foo()
@@ -17,4 +23,16 @@ int foo()
     return always_inline_chain0();
 }
 
-// CHECK-NOT: define
+// CHECK-NOT: always_inline_chain
+// CHECK-NOT: call_template_foo
+
+// CHECK: define {{.*}}_D15inlining_gh31263barFZi
+int bar()
+{
+    // no calls to [call_]template_foo
+    // CHECK-NOT: call {{.*}}template_foo
+    return call_template_foo(123);
+}
+
+// CHECK-NOT: always_inline_chain
+// CHECK-NOT: call_template_foo