[Modules TS] Module ownership semantics for redeclarations.

When declaring an entity in the "purview" of a module, it's never a
redeclaration of an entity in the purview of a default module or in no module
("in the global module"). Don't consider those other declarations as possible
redeclaration targets if they're not visible, and reject any cases where we
pick a prior visible declaration that violates this rule.

This reinstates r315251 and r315256, reverted in r315309 and r315308
respectively, tweaked to avoid triggering a linkage calculation when declaring
implicit special members (this exposed our pre-existing issue with typedef
names for linkage changing the linkage of types whose linkage has already been
computed and cached in more cases). A testcase for that regression has been
added in r315366.


git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@315379 91177308-0d34-0410-b5e6-96231b3b80d8

Author: zygoloid

include/clang/AST/Decl.h

@@ -339,6 +339,12 @@ class NamedDecl : public Decl {
     return clang::isExternallyVisible(getLinkageInternal());
   }
 
+  /// Determine whether this declaration can be redeclared in a
+  /// different translation unit.
+  bool isExternallyDeclarable() const {
+    return isExternallyVisible() && !getOwningModuleForLinkage();
+  }
+
   /// \brief Determines the visibility of this entity.
   Visibility getVisibility() const {
     return getLinkageAndVisibility().getVisibility();
@@ -379,10 +385,6 @@ class NamedDecl : public Decl {
     return hasCachedLinkage();
   }
 
-  /// Get the module that owns this declaration for linkage purposes.
-  /// There only ever is such a module under the C++ Modules TS.
-  Module *getOwningModuleForLinkage() const;
-
   /// \brief Looks through UsingDecls and ObjCCompatibleAliasDecls for
   /// the underlying named decl.
   NamedDecl *getUnderlyingDecl() {

include/clang/AST/DeclBase.h

@@ -738,6 +738,13 @@ class LLVM_ALIGNAS(/*alignof(uint64_t)*/ 8) Decl {
     return isFromASTFile() ? getImportedOwningModule() : getLocalOwningModule();
   }
 
+  /// Get the module that owns this declaration for linkage purposes.
+  /// There only ever is such a module under the C++ Modules TS.
+  ///
+  /// \param IgnoreLinkage Ignore the linkage of the entity; assume that
+  /// all declarations in a global module fragment are unowned.
+  Module *getOwningModuleForLinkage(bool IgnoreLinkage = false) const;
+
   /// \brief Determine whether this declaration might be hidden from name
   /// lookup. Note that the declaration might be visible even if this returns
   /// \c false, if the owning module is visible within the query context.

include/clang/Basic/DiagnosticSemaKinds.td

@@ -4801,6 +4801,9 @@ def err_thread_non_thread : Error<
 def err_thread_thread_different_kind : Error<
   "thread-local declaration of %0 with %select{static|dynamic}1 initialization "
   "follows declaration with %select{dynamic|static}1 initialization">;
+def err_mismatched_owning_module : Error<
+  "declaration of %0 in %select{the global module|module %2}1 follows "
+  "declaration in %select{the global module|module %4}3">;
 def err_redefinition_different_type : Error<
   "redefinition of %0 with a different type%diff{: $ vs $|}1,2">;
 def err_redefinition_different_kind : Error<

include/clang/Sema/Lookup.h

@@ -139,6 +139,7 @@ class LookupResult {
       LookupKind(LookupKind),
       IDNS(0),
       Redecl(Redecl != Sema::NotForRedeclaration),
+      ExternalRedecl(Redecl == Sema::ForExternalRedeclaration),
       HideTags(true),
       Diagnose(Redecl == Sema::NotForRedeclaration),
       AllowHidden(false),
@@ -161,6 +162,7 @@ class LookupResult {
       LookupKind(LookupKind),
       IDNS(0),
       Redecl(Redecl != Sema::NotForRedeclaration),
+      ExternalRedecl(Redecl == Sema::ForExternalRedeclaration),
       HideTags(true),
       Diagnose(Redecl == Sema::NotForRedeclaration),
       AllowHidden(false),
@@ -181,6 +183,7 @@ class LookupResult {
       LookupKind(Other.LookupKind),
       IDNS(Other.IDNS),
       Redecl(Other.Redecl),
+      ExternalRedecl(Other.ExternalRedecl),
       HideTags(Other.HideTags),
       Diagnose(false),
       AllowHidden(Other.AllowHidden),
@@ -201,7 +204,9 @@ class LookupResult {
         SemaPtr(std::move(Other.SemaPtr)), NameInfo(std::move(Other.NameInfo)),
         NameContextRange(std::move(Other.NameContextRange)),
         LookupKind(std::move(Other.LookupKind)), IDNS(std::move(Other.IDNS)),
-        Redecl(std::move(Other.Redecl)), HideTags(std::move(Other.HideTags)),
+        Redecl(std::move(Other.Redecl)),
+        ExternalRedecl(std::move(Other.ExternalRedecl)),
+        HideTags(std::move(Other.HideTags)),
         Diagnose(std::move(Other.Diagnose)),
         AllowHidden(std::move(Other.AllowHidden)),
         Shadowed(std::move(Other.Shadowed)) {
@@ -221,6 +226,7 @@ class LookupResult {
     LookupKind = std::move(Other.LookupKind);
     IDNS = std::move(Other.IDNS);
     Redecl = std::move(Other.Redecl);
+    ExternalRedecl = std::move(Other.ExternalRedecl);
     HideTags = std::move(Other.HideTags);
     Diagnose = std::move(Other.Diagnose);
     AllowHidden = std::move(Other.AllowHidden);
@@ -265,6 +271,17 @@ class LookupResult {
     return Redecl;
   }
 
+  /// True if this lookup is just looking for an existing declaration to link
+  /// against a declaration with external linkage.
+  bool isForExternalRedeclaration() const {
+    return ExternalRedecl;
+  }
+
+  Sema::RedeclarationKind redeclarationKind() const {
+    return ExternalRedecl ? Sema::ForExternalRedeclaration :
+           Redecl ? Sema::ForVisibleRedeclaration : Sema::NotForRedeclaration;
+  }
+
   /// \brief Specify whether hidden declarations are visible, e.g.,
   /// for recovery reasons.
   void setAllowHidden(bool AH) {
@@ -275,7 +292,7 @@ class LookupResult {
   /// declarations, such as those in modules that have not yet been imported.
   bool isHiddenDeclarationVisible(NamedDecl *ND) const {
     return AllowHidden ||
-           (isForRedeclaration() && ND->hasExternalFormalLinkage());
+           (isForExternalRedeclaration() && ND->isExternallyDeclarable());
   }
 
   /// Sets whether tag declarations should be hidden by non-tag
@@ -556,7 +573,8 @@ class LookupResult {
 
   /// \brief Change this lookup's redeclaration kind.
   void setRedeclarationKind(Sema::RedeclarationKind RK) {
-    Redecl = RK;
+    Redecl = (RK != Sema::NotForRedeclaration);
+    ExternalRedecl = (RK == Sema::ForExternalRedeclaration);
     configure();
   }
 
@@ -719,6 +737,7 @@ class LookupResult {
   unsigned IDNS; // set by configure()
 
   bool Redecl;
+  bool ExternalRedecl;
 
   /// \brief True if tag declarations should be hidden if non-tags
   ///   are present

include/clang/Sema/Sema.h

@@ -285,15 +285,21 @@ class Sema {
 
   bool isVisibleSlow(const NamedDecl *D);
 
+  /// Determine whether two declarations should be linked together, given that
+  /// the old declaration might not be visible and the new declaration might
+  /// not have external linkage.
   bool shouldLinkPossiblyHiddenDecl(const NamedDecl *Old,
                                     const NamedDecl *New) {
-    // We are about to link these. It is now safe to compute the linkage of
-    // the new decl. If the new decl has external linkage, we will
-    // link it with the hidden decl (which also has external linkage) and
-    // it will keep having external linkage. If it has internal linkage, we
-    // will not link it. Since it has no previous decls, it will remain
-    // with internal linkage.
-    return isVisible(Old) || New->isExternallyVisible();
+    if (isVisible(Old))
+     return true;
+    // See comment in below overload for why it's safe to compute the linkage
+    // of the new declaration here.
+    if (New->isExternallyDeclarable()) {
+      assert(Old->isExternallyDeclarable() &&
+             "should not have found a non-externally-declarable previous decl");
+      return true;
+    }
+    return false;
   }
   bool shouldLinkPossiblyHiddenDecl(LookupResult &Old, const NamedDecl *New);
 
@@ -3035,10 +3041,25 @@ class Sema {
     /// purpose of redeclaring the name.
     NotForRedeclaration = 0,
     /// \brief The lookup results will be used for redeclaration of a name,
-    /// if an entity by that name already exists.
-    ForRedeclaration
+    /// if an entity by that name already exists and is visible.
+    ForVisibleRedeclaration,
+    /// \brief The lookup results will be used for redeclaration of a name
+    /// with external linkage; non-visible lookup results with external linkage
+    /// may also be found.
+    ForExternalRedeclaration
   };
 
+  RedeclarationKind forRedeclarationInCurContext() {
+    // A declaration with an owning module for linkage can never link against
+    // anything that is not visible. We don't need to check linkage here; if
+    // the context has internal linkage, redeclaration lookup won't find things
+    // from other TUs, and we can't safely compute linkage yet in general.
+    if (cast<Decl>(CurContext)
+            ->getOwningModuleForLinkage(/*IgnoreLinkage*/true))
+      return ForVisibleRedeclaration;
+    return ForExternalRedeclaration;
+  }
+
   /// \brief The possible outcomes of name lookup for a literal operator.
   enum LiteralOperatorLookupResult {
     /// \brief The lookup resulted in an error.
@@ -3266,6 +3287,8 @@ class Sema {
   void FilterLookupForScope(LookupResult &R, DeclContext *Ctx, Scope *S,
                             bool ConsiderLinkage, bool AllowInlineNamespace);
 
+  bool CheckRedeclarationModuleOwnership(NamedDecl *New, NamedDecl *Old);
+
   void DiagnoseAmbiguousLookup(LookupResult &Result);
   //@}
 

lib/AST/Decl.cpp

@@ -515,6 +515,7 @@ static bool isSingleLineLanguageLinkage(const Decl &D) {
 }
 
 static bool isExportedFromModuleIntefaceUnit(const NamedDecl *D) {
+  // FIXME: Handle isModulePrivate.
   switch (D->getModuleOwnershipKind()) {
   case Decl::ModuleOwnershipKind::Unowned:
   case Decl::ModuleOwnershipKind::ModulePrivate:
@@ -546,7 +547,8 @@ static LinkageInfo getExternalLinkageFor(const NamedDecl *D) {
   //     declaration has module linkage.
   if (auto *M = D->getOwningModule())
     if (M->Kind == Module::ModuleInterfaceUnit)
-      if (!isExportedFromModuleIntefaceUnit(D))
+      if (!isExportedFromModuleIntefaceUnit(
+              cast<NamedDecl>(D->getCanonicalDecl())))
         return LinkageInfo(ModuleLinkage, DefaultVisibility, false);
 
   return LinkageInfo::external();
@@ -1393,7 +1395,7 @@ LinkageInfo LinkageComputer::getDeclLinkageAndVisibility(const NamedDecl *D) {
                                             : NamedDecl::VisibilityForValue));
 }
 
-Module *NamedDecl::getOwningModuleForLinkage() const {
+Module *Decl::getOwningModuleForLinkage(bool IgnoreLinkage) const {
   Module *M = getOwningModule();
   if (!M)
     return nullptr;
@@ -1411,7 +1413,17 @@ Module *NamedDecl::getOwningModuleForLinkage() const {
     // for linkage purposes. But internal linkage declarations in the global
     // module fragment of a particular module are owned by that module for
     // linkage purposes.
-    return hasExternalFormalLinkage() ? nullptr : M->Parent;
+    if (IgnoreLinkage)
+      return nullptr;
+    bool InternalLinkage;
+    if (auto *ND = dyn_cast<NamedDecl>(this))
+      InternalLinkage = !ND->hasExternalFormalLinkage();
+    else {
+      auto *NSD = dyn_cast<NamespaceDecl>(this);
+      InternalLinkage = (NSD && NSD->isAnonymousNamespace()) ||
+                        isInAnonymousNamespace();
+    }
+    return InternalLinkage ? M->Parent : nullptr;
   }
 
   llvm_unreachable("unknown module kind");

lib/AST/DeclBase.cpp

@@ -315,12 +315,11 @@ bool Decl::isLexicallyWithinFunctionOrMethod() const {
 }
 
 bool Decl::isInAnonymousNamespace() const {
-  const DeclContext *DC = getDeclContext();
-  do {
+  for (const DeclContext *DC = getDeclContext(); DC; DC = DC->getParent()) {
     if (const NamespaceDecl *ND = dyn_cast<NamespaceDecl>(DC))
       if (ND->isAnonymousNamespace())
         return true;
-  } while ((DC = DC->getParent()));
+  }
 
   return false;
 }

lib/AST/DeclCXX.cpp

@@ -100,7 +100,7 @@ CXXRecordDecl::CXXRecordDecl(Kind K, TagKind TK, const ASTContext &C,
 CXXRecordDecl *CXXRecordDecl::Create(const ASTContext &C, TagKind TK,
                                      DeclContext *DC, SourceLocation StartLoc,
                                      SourceLocation IdLoc, IdentifierInfo *Id,
-                                     CXXRecordDecl* PrevDecl,
+                                     CXXRecordDecl *PrevDecl,
                                      bool DelayTypeCreation) {
   CXXRecordDecl *R = new (C, DC) CXXRecordDecl(CXXRecord, TK, C, DC, StartLoc,
                                                IdLoc, Id, PrevDecl);