Implement C++ [basic.link]p8.

If a function or variable has a type with no linkage (and is not extern "C"),
any use of it requires a definition within the same translation unit; the idea
is that it is not possible to define the entity elsewhere, so any such use is
necessarily an error.

There is an exception, though: some types formally have no linkage but
nonetheless can be referenced from other translation units (for example, this
happens to anonymous structures defined within inline functions). For entities
with those types, we suppress the diagnostic except under -pedantic.

llvm-svn: 313729

15 files changed, 180 insertions, 56 deletions

diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 3974fe0eea5..16fee891302 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -4707,6 +4707,14 @@ def note_deleted_assign_field : Note<
 def warn_undefined_internal : Warning<
   "%select{function|variable}0 %q1 has internal linkage but is not defined">,
   InGroup<DiagGroup<"undefined-internal">>;
+def err_undefined_internal_type : Error<
+  "%select{function|variable}0 %q1 is used but not defined in this "
+  "translation unit, and cannot be defined in any other translation unit "
+  "because its type does not have linkage">;
+def ext_undefined_internal_type : Extension<
+  "ISO C++ requires a definition in this translation unit for "
+  "%select{function|variable}0 %q1 because its type does not have linkage">,
+  InGroup<DiagGroup<"undefined-internal-type">>;
 def warn_undefined_inline : Warning<"inline function %q0 is not defined">,
   InGroup<DiagGroup<"undefined-inline">>;
 def err_undefined_inline_var : Error<"inline variable %q0 is not defined">;
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index fc34f6816f8..9420407acb8 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -1086,6 +1086,11 @@ public:
   /// definition in this translation unit.
   llvm::MapVector<NamedDecl *, SourceLocation> UndefinedButUsed;
 
+  /// Determine if VD, which must be a variable or function, is an external
+  /// symbol that nonetheless can't be referenced from outside this translation
+  /// unit because its type has no linkage and it's not extern "C".
+  bool isExternalWithNoLinkageType(ValueDecl *VD);
+
   /// Obtain a sorted list of functions that are undefined but ODR-used.
   void getUndefinedButUsed(
       SmallVectorImpl<std::pair<NamedDecl *, SourceLocation> > &Undefined);
diff --git a/clang/include/clang/Sema/SemaInternal.h b/clang/include/clang/Sema/SemaInternal.h
index a01e8d639f6..4dc215ba21c 100644
--- a/clang/include/clang/Sema/SemaInternal.h
+++ b/clang/include/clang/Sema/SemaInternal.h
@@ -73,7 +73,8 @@ inline void MarkVarDeclODRUsed(VarDecl *Var,
   // Keep track of used but undefined variables.
   // FIXME: We shouldn't suppress this warning for static data members.
   if (Var->hasDefinition(SemaRef.Context) == VarDecl::DeclarationOnly &&
-      (!Var->isExternallyVisible() || Var->isInline()) &&
+      (!Var->isExternallyVisible() || Var->isInline() ||
+       SemaRef.isExternalWithNoLinkageType(Var)) &&
       !(Var->isStaticDataMember() && Var->hasInit())) {
     SourceLocation &old = SemaRef.UndefinedButUsed[Var->getCanonicalDecl()];
     if (old.isInvalid())
diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index 0a4c4a4a776..c3e3de1e22b 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -721,8 +721,7 @@ LinkageComputer::getLVForNamespaceScopeDecl(const NamedDecl *D,
     // because of this, but unique-external linkage suits us.
     if (Context.getLangOpts().CPlusPlus && !isFirstInExternCContext(Var)) {
       LinkageInfo TypeLV = getLVForType(*Var->getType(), computation);
-      if (TypeLV.getLinkage() != ExternalLinkage &&
-          TypeLV.getLinkage() != ModuleLinkage)
+      if (!isExternallyVisible(TypeLV.getLinkage()))
         return LinkageInfo::uniqueExternal();
       if (!LV.isVisibilityExplicit())
         LV.mergeVisibility(TypeLV);
@@ -772,7 +771,7 @@ LinkageComputer::getLVForNamespaceScopeDecl(const NamedDecl *D,
       QualType TypeAsWritten = Function->getType();
       if (TypeSourceInfo *TSI = Function->getTypeSourceInfo())
         TypeAsWritten = TSI->getType();
-      if (TypeAsWritten->getLinkage() == UniqueExternalLinkage)
+      if (!isExternallyVisible(TypeAsWritten->getLinkage()))
         return LinkageInfo::uniqueExternal();
     }
 
@@ -909,7 +908,7 @@ LinkageComputer::getLVForClassMember(const NamedDecl *D,
   const NamedDecl *explicitSpecSuppressor = nullptr;
 
   if (const auto *MD = dyn_cast<CXXMethodDecl>(D)) {
-    // If the type of the function uses a type with unique-external
+    // If the type of the function uses a type that has non-externally-visible
     // linkage, it's not legally usable from outside this translation unit.
     // But only look at the type-as-written. If this function has an
     // auto-deduced return type, we can't compute the linkage of that type
@@ -922,7 +921,7 @@ LinkageComputer::getLVForClassMember(const NamedDecl *D,
       QualType TypeAsWritten = MD->getType();
       if (TypeSourceInfo *TSI = MD->getTypeSourceInfo())
         TypeAsWritten = TSI->getType();
-      if (TypeAsWritten->getLinkage() == UniqueExternalLinkage)
+      if (!isExternallyVisible(TypeAsWritten->getLinkage()))
         return LinkageInfo::uniqueExternal();
     }
     // If this is a method template specialization, use the linkage for
@@ -1119,6 +1118,9 @@ LinkageInfo LinkageComputer::getLVForClosure(const DeclContext *DC,
                                              Decl *ContextDecl,
                                              LVComputationKind computation) {
   // This lambda has its linkage/visibility determined by its owner.
+  // FIXME: This is wrong. A lambda never formally has linkage; if this
+  // calculation determines the lambda has external linkage, it should be
+  // downgraded to VisibleNoLinkage.
   if (ContextDecl) {
     if (isa<ParmVarDecl>(ContextDecl))
       DC = ContextDecl->getDeclContext()->getRedeclContext();
diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp
index 66cb4976dac..62692519674 100644
--- a/clang/lib/Sema/Sema.cpp
+++ b/clang/lib/Sema/Sema.cpp
@@ -582,6 +582,23 @@ static bool ShouldRemoveFromUnused(Sema *SemaRef, const DeclaratorDecl *D) {
   return false;
 }
 
+static bool isFunctionOrVarDeclExternC(NamedDecl *ND) {
+  if (auto *FD = dyn_cast<FunctionDecl>(ND))
+    return FD->isExternC();
+  return cast<VarDecl>(ND)->isExternC();
+}
+
+/// Determine whether ND is an external-linkage function or variable whose
+/// type has no linkage.
+bool Sema::isExternalWithNoLinkageType(ValueDecl *VD) {
+  // Note: it's not quite enough to check whether VD has UniqueExternalLinkage,
+  // because we also want to catch the case where its type has VisibleNoLinkage,
+  // which does not affect the linkage of VD.
+  return getLangOpts().CPlusPlus && VD->hasExternalFormalLinkage() &&
+         !isExternalFormalLinkage(VD->getType()->getLinkage()) &&
+         !isFunctionOrVarDeclExternC(VD);
+}
+
 /// Obtains a sorted list of functions and variables that are undefined but
 /// ODR-used.
 void Sema::getUndefinedButUsed(
@@ -598,17 +615,27 @@ void Sema::getUndefinedButUsed(
     if (isa<CXXDeductionGuideDecl>(ND))
       continue;
 
+    if (ND->hasAttr<DLLImportAttr>() || ND->hasAttr<DLLExportAttr>()) {
+      // An exported function will always be emitted when defined, so even if
+      // the function is inline, it doesn't have to be emitted in this TU. An
+      // imported function implies that it has been exported somewhere else.
+      continue;
+    }
+
     if (FunctionDecl *FD = dyn_cast<FunctionDecl>(ND)) {
       if (FD->isDefined())
         continue;
       if (FD->isExternallyVisible() &&
+          !isExternalWithNoLinkageType(FD) &&
           !FD->getMostRecentDecl()->isInlined())
         continue;
     } else {
       auto *VD = cast<VarDecl>(ND);
       if (VD->hasDefinition() != VarDecl::DeclarationOnly)
         continue;
-      if (VD->isExternallyVisible() && !VD->getMostRecentDecl()->isInline())
+      if (VD->isExternallyVisible() &&
+          !isExternalWithNoLinkageType(VD) &&
+          !VD->getMostRecentDecl()->isInline())
         continue;
     }
 
@@ -626,33 +653,43 @@ static void checkUndefinedButUsed(Sema &S) {
   S.getUndefinedButUsed(Undefined);
   if (Undefined.empty()) return;
 
-  for (SmallVectorImpl<std::pair<NamedDecl *, SourceLocation> >::iterator
-         I = Undefined.begin(), E = Undefined.end(); I != E; ++I) {
-    NamedDecl *ND = I->first;
-
-    if (ND->hasAttr<DLLImportAttr>() || ND->hasAttr<DLLExportAttr>()) {
-      // An exported function will always be emitted when defined, so even if
-      // the function is inline, it doesn't have to be emitted in this TU. An
-      // imported function implies that it has been exported somewhere else.
-      continue;
-    }
-
-    if (!ND->isExternallyVisible()) {
-      S.Diag(ND->getLocation(), diag::warn_undefined_internal)
-        << isa<VarDecl>(ND) << ND;
-    } else if (auto *FD = dyn_cast<FunctionDecl>(ND)) {
+  for (auto Undef : Undefined) {
+    ValueDecl *VD = cast<ValueDecl>(Undef.first);
+    SourceLocation UseLoc = Undef.second;
+
+    if (S.isExternalWithNoLinkageType(VD)) {
+      // C++ [basic.link]p8:
+      //   A type without linkage shall not be used as the type of a variable
+      //   or function with external linkage unless
+      //    -- the entity has C language linkage
+      //    -- the entity is not odr-used or is defined in the same TU
+      //
+      // As an extension, accept this in cases where the type is externally
+      // visible, since the function or variable actually can be defined in
+      // another translation unit in that case.
+      S.Diag(VD->getLocation(), isExternallyVisible(VD->getType()->getLinkage())
+                                    ? diag::ext_undefined_internal_type
+                                    : diag::err_undefined_internal_type)
+        << isa<VarDecl>(VD) << VD;
+    } else if (!VD->isExternallyVisible()) {
+      // FIXME: We can promote this to an error. The function or variable can't
+      // be defined anywhere else, so the program must necessarily violate the
+      // one definition rule.
+      S.Diag(VD->getLocation(), diag::warn_undefined_internal)
+        << isa<VarDecl>(VD) << VD;
+    } else if (auto *FD = dyn_cast<FunctionDecl>(VD)) {
       (void)FD;
       assert(FD->getMostRecentDecl()->isInlined() &&
              "used object requires definition but isn't inline or internal?");
       // FIXME: This is ill-formed; we should reject.
-      S.Diag(ND->getLocation(), diag::warn_undefined_inline) << ND;
+      S.Diag(VD->getLocation(), diag::warn_undefined_inline) << VD;
     } else {
-      assert(cast<VarDecl>(ND)->getMostRecentDecl()->isInline() &&
+      assert(cast<VarDecl>(VD)->getMostRecentDecl()->isInline() &&
              "used var requires definition but isn't inline or internal?");
-      S.Diag(ND->getLocation(), diag::err_undefined_inline_var) << ND;
+      S.Diag(VD->getLocation(), diag::err_undefined_inline_var) << VD;
     }
-    if (I->second.isValid())
-      S.Diag(I->second, diag::note_used_here);
+    if (UseLoc.isValid())
+      S.Diag(UseLoc, diag::note_used_here);
   }
 
   S.UndefinedButUsed.clear();
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 2cd93364869..389cd7ab737 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -3827,7 +3827,7 @@ void Sema::MergeVarDecl(VarDecl *New, LookupResult &Previous) {
     }
   }
 
-  // If this redeclaration makes the function inline, we may need to add it to
+  // If this redeclaration makes the variable inline, we may need to add it to
   // UndefinedButUsed.
   if (!Old->isInline() && New->isInline() && Old->isUsed(false) &&
       !Old->getDefinition() && !New->isThisDeclarationADefinition())
@@ -12329,18 +12329,6 @@ Decl *Sema::ActOnFinishFunctionBody(Decl *dcl, Stmt *Body,
       }
     }
 
-    // The only way to be included in UndefinedButUsed is if there is an
-    // ODR use before the definition. Avoid the expensive map lookup if this
-    // is the first declaration.
-    if (!FD->isFirstDecl() && FD->getPreviousDecl()->isUsed()) {
-      if (!FD->isExternallyVisible())
-        UndefinedButUsed.erase(FD);
-      else if (FD->isInlined() &&
-               !LangOpts.GNUInline &&
-               (!FD->getPreviousDecl()->hasAttr<GNUInlineAttr>()))
-        UndefinedButUsed.erase(FD);
-    }
-
     // If the function implicitly returns zero (like 'main') or is naked,
     // don't complain about missing return statements.
     if (FD->hasImplicitReturnZero() || FD->hasAttr<NakedAttr>())
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index c1b2ad0d738..46853dd32c5 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -13897,6 +13897,8 @@ void Sema::MarkFunctionReferenced(SourceLocation Loc, FunctionDecl *Func,
              !LangOpts.GNUInline &&
              !Func->getMostRecentDecl()->hasAttr<GNUInlineAttr>())
       UndefinedButUsed.insert(std::make_pair(Func->getCanonicalDecl(), Loc));
+    else if (isExternalWithNoLinkageType(Func))
+      UndefinedButUsed.insert(std::make_pair(Func->getCanonicalDecl(), Loc));
   }
 
   Func->markUsed(Context);
diff --git a/clang/test/Analysis/malloc.mm b/clang/test/Analysis/malloc.mm
index f8a43b3b6a9..e3daa858be8 100644
--- a/clang/test/Analysis/malloc.mm
+++ b/clang/test/Analysis/malloc.mm
@@ -187,7 +187,7 @@ typedef volatile struct {
  void *opaque1;
  long opaque2;
 } OSQueueHead;
-void OSAtomicEnqueue( OSQueueHead *__list, void *__new, size_t __offset) __attribute__((weak_import));
+extern "C" void OSAtomicEnqueue( OSQueueHead *__list, void *__new, size_t __offset) __attribute__((weak_import));
 static inline void radar11111210(OSQueueHead *pool) {
     void *newItem = malloc(4);
     OSAtomicEnqueue(pool, newItem, 4);
diff --git a/clang/test/Analysis/reference.cpp b/clang/test/Analysis/reference.cpp
index b323b966610..c269dd7bad1 100644
--- a/clang/test/Analysis/reference.cpp
+++ b/clang/test/Analysis/reference.cpp
@@ -117,6 +117,11 @@ void testRetroactiveNullReference(int *x) {
   y = 5; // expected-warning{{Dereference of null pointer}}
 }
 
+namespace TestReferenceAddress {
+struct S { int &x; };
+S getS();
+S *getSP();
+
 void testReferenceAddress(int &x) {
 // FIXME: Move non-zero reference assumption out of RangeConstraintManager.cpp:422
 #ifdef ANALYZER_CM_Z3
@@ -127,23 +132,19 @@ void testReferenceAddress(int &x) {
   clang_analyzer_eval(&ref() != 0); // expected-warning{{TRUE}}
 #endif
 
-  struct S { int &x; };
-
-  extern S getS();
 #ifdef ANALYZER_CM_Z3
   clang_analyzer_eval(&getS().x != 0); // expected-warning{{UNKNOWN}}
 #else
   clang_analyzer_eval(&getS().x != 0); // expected-warning{{TRUE}}
 #endif
 
-  extern S *getSP();
 #ifdef ANALYZER_CM_Z3
   clang_analyzer_eval(&getSP()->x != 0); // expected-warning{{UNKNOWN}}
 #else
   clang_analyzer_eval(&getSP()->x != 0); // expected-warning{{TRUE}}
 #endif
 }
-
+}
 
 void testFunctionPointerReturn(void *opaque) {
   typedef int &(*RefFn)();
diff --git a/clang/test/CXX/basic/basic.link/p8.cpp b/clang/test/CXX/basic/basic.link/p8.cpp
new file mode 100644
index 00000000000..842a2414b63
--- /dev/null
+++ b/clang/test/CXX/basic/basic.link/p8.cpp
@@ -0,0 +1,69 @@
+// RUN: %clang_cc1 -std=c++2a -verify %s -pedantic
+
+template<typename T> struct Template {};
+
+struct Linkage1 { struct Inner {}; };
+typedef struct { struct Inner {}; } Linkage2;
+
+typedef struct {} const NoLinkage1;
+auto x = [] {};
+typedef decltype(x) NoLinkage2;
+auto f() { return [] {}; }
+typedef decltype(f()) NoLinkage3;
+
+inline auto g() { return [] {}; }
+typedef decltype(g()) VisibleNoLinkage1;
+inline auto y = [] {};
+typedef decltype(x) VisibleNoLinkage2;
+inline auto h() { struct {} x; return x; }
+typedef decltype(h()) VisibleNoLinkage3;
+
+extern Linkage1 linkage1v;
+extern Linkage1::Inner linkage1iv;
+extern Linkage2 linkage2v;
+extern Linkage2::Inner linkage2iv;
+extern Template<Linkage1> linkaget1v;
+extern Linkage1 linkage1f();
+void linkage2f(Linkage2);
+
+void use_linkage() {
+  &linkage1v, &linkage1iv, &linkage2v, &linkage2iv, &linkaget1v; // expected-warning 5{{unused}}
+  linkage1f();
+  linkage2f({});
+}
+
+extern NoLinkage1 no_linkage1(); // expected-error {{function 'no_linkage1' is used but not defined in this translation unit}}
+extern NoLinkage2 no_linkage2(); // expected-error {{function 'no_linkage2' is used but not defined in this translation unit}}
+extern NoLinkage3 no_linkage3(); // expected-error {{function 'no_linkage3' is used but not defined in this translation unit}}
+
+void use_no_linkage() {
+  no_linkage1(); // expected-note {{used here}}
+  no_linkage2(); // expected-note {{used here}}
+  no_linkage3(); // expected-note {{used here}}
+}
+
+// FIXME: This should emit an extension warning. It does not because we
+// incorrectly give the lambda external linkage.
+extern VisibleNoLinkage1 visible_no_linkage1();
+
+// FIXME: We should accept this as an extension. We don't because we
+// incorrectly give the lambda no linkage instead of "VisibleNoLinkage".
+extern VisibleNoLinkage2 visible_no_linkage2(); // expected-error {{used but not defined}}
+
+// This case is correctly accepted as an extension.
+extern VisibleNoLinkage3 visible_no_linkage3(); // expected-warning {{ISO C++ requires a definition}}
+
+void use_visible_no_linkage() {
+  visible_no_linkage1();
+  visible_no_linkage2(); // expected-note {{used here}}
+  visible_no_linkage3(); // expected-note {{used here}}
+}
+
+
+extern inline int not_defined; // expected-error {{not defined}}
+extern inline int defined_after_use;
+void use_inline_vars() {
+  not_defined = 1; // expected-note {{used here}}
+  defined_after_use = 2;
+}
+inline int defined_after_use;
diff --git a/clang/test/CodeGenCXX/debug-info-method.cpp b/clang/test/CodeGenCXX/debug-info-method.cpp
index e0f9a284f2c..af7103e5752 100644
--- a/clang/test/CodeGenCXX/debug-info-method.cpp
+++ b/clang/test/CodeGenCXX/debug-info-method.cpp
@@ -20,6 +20,7 @@ union {
 class A {
 protected:
   void foo(int, A, decltype(u));
+  void bar();
 }; 
 
 void A::foo(int, A, decltype(u)) {
@@ -29,3 +30,5 @@ A a;
 
 int A::*x = 0;
 int (A::*y)(int) = 0;
+
+void A::bar() { foo(0, *this, u); }
diff --git a/clang/test/CodeGenCXX/mangle-ms-cxx11.cpp b/clang/test/CodeGenCXX/mangle-ms-cxx11.cpp
index 819b0d9c9d0..b22c04698ea 100644
--- a/clang/test/CodeGenCXX/mangle-ms-cxx11.cpp
+++ b/clang/test/CodeGenCXX/mangle-ms-cxx11.cpp
@@ -160,6 +160,8 @@ struct { } a;
 decltype(a) fun(decltype(a) x, decltype(a)) { return x; }
 // CHECK-DAG: @"\01?fun@PR18022@@YA?AU<unnamed-type-a>@1@U21@0@Z"
 
+void use_fun() { fun(a, a); }
+
 }
 
 inline int define_lambda() {
@@ -280,7 +282,7 @@ void g() {
 
 namespace PR18204 {
 template <typename T>
-int f(T *);
+int f(T *) { return 0; }
 static union {
   int n = f(this);
 };
@@ -346,4 +348,5 @@ int call_it = (A::default_args(), 1);
 
 enum { enumerator };
 void f(decltype(enumerator)) {}
-// CHECK-DAG: define void @"\01?f@@YAXW4<unnamed-enum-enumerator>@@@Z"(
+// CHECK-DAG: define internal void @"\01?f@@YAXW4<unnamed-enum-enumerator>@@@Z"(
+void use_f() { f(enumerator); }
diff --git a/clang/test/CodeGenCXX/mangle.cpp b/clang/test/CodeGenCXX/mangle.cpp
index 91fe6aeef29..919f8afb160 100644
--- a/clang/test/CodeGenCXX/mangle.cpp
+++ b/clang/test/CodeGenCXX/mangle.cpp
@@ -893,7 +893,8 @@ namespace test39 {
     struct {} a;
   } *foo;
   template<typename T> void func(T) {}
-  void test(foo x) {
+  void test() {
+    foo x;
     func(x->a);
   }
 }
diff --git a/clang/test/PCH/cxx11-lambdas.mm b/clang/test/PCH/cxx11-lambdas.mm
index 1f568f05e11..9628d12aa15 100644
--- a/clang/test/PCH/cxx11-lambdas.mm
+++ b/clang/test/PCH/cxx11-lambdas.mm
@@ -39,7 +39,7 @@ int init_capture(T t) {
 }
 
 struct X {
-  template <typename T> X(T);
+  template <typename T> X(T) {}
 };
 struct Y { Y(const X &x = [] {}); };
 
diff --git a/clang/test/SemaCXX/warn-unreachable.cpp b/clang/test/SemaCXX/warn-unreachable.cpp
index b08467ab51e..08118147b75 100644
--- a/clang/test/SemaCXX/warn-unreachable.cpp
+++ b/clang/test/SemaCXX/warn-unreachable.cpp
@@ -49,22 +49,26 @@ void test3() {
       (halt()); // expected-warning {{will never be executed}}
 }
 
-void test4() {
+namespace Test4 {
   struct S {
     int mem;
   } s;
   S &foor();
-  halt(), foor()// expected-warning {{will never be executed}}
-    .mem;       
+  void test4() {
+    halt(), foor()// expected-warning {{will never be executed}}
+      .mem;       
+  }
 }
 
-void test5() {
+namespace Test5 {
   struct S {
     int mem;
   } s;
   S &foonr() __attribute__((noreturn));
-  foonr()
-    .mem;       // expected-warning {{will never be executed}}
+  void test5() {
+    foonr()
+      .mem;       // expected-warning {{will never be executed}}
+  }
 }
 
 void test6() {