[modules] When considering merging a newly-declared typedef into an imported

one, perform the import if the types match even if the imported declaration is
hidden. Otherwise, NamedDecl::declarationReplaces will drop one of the name
lookup entries, making the typedef effectively inaccessible from one of the
modules that declared it.

llvm-svn: 215306

Author: zygoloid

clang/lib/Sema/SemaDecl.cpp

@@ -1725,6 +1725,43 @@ static void filterNonConflictingPreviousDecls(ASTContext &context,
   filter.done();
 }
 
+/// Typedef declarations don't have linkage, but they still denote the same
+/// entity if their types are the same.
+/// FIXME: This is notionally doing the same thing as ASTReaderDecl's
+/// isSameEntity.
+static void filterNonConflictingPreviousTypedefDecls(ASTContext &Context,
+                                                     TypedefNameDecl *Decl,
+                                                     LookupResult &Previous) {
+  // This is only interesting when modules are enabled.
+  if (!Context.getLangOpts().Modules)
+    return;
+
+  // Empty sets are uninteresting.
+  if (Previous.empty())
+    return;
+
+  LookupResult::Filter Filter = Previous.makeFilter();
+  while (Filter.hasNext()) {
+    NamedDecl *Old = Filter.next();
+
+    // Non-hidden declarations are never ignored.
+    if (!Old->isHidden())
+      continue;
+
+    // Declarations of the same entity are not ignored, even if they have
+    // different linkages.
+    if (auto *OldTD = dyn_cast<TypedefNameDecl>(Old))
+      if (Context.hasSameType(OldTD->getUnderlyingType(),
+                              Decl->getUnderlyingType()))
+        continue;
+
+    if (!Old->isExternallyVisible())
+      Filter.erase();
+  }
+
+  Filter.done();
+}
+
 bool Sema::isIncompatibleTypedef(TypeDecl *Old, TypedefNameDecl *New) {
   QualType OldType;
   if (TypedefNameDecl *OldTypedef = dyn_cast<TypedefNameDecl>(Old))
@@ -4835,7 +4872,7 @@ Sema::ActOnTypedefNameDecl(Scope *S, DeclContext *DC, TypedefNameDecl *NewTD,
   // in an outer scope, it isn't the same thing.
   FilterLookupForScope(Previous, DC, S, /*ConsiderLinkage*/false,
                        /*AllowInlineNamespace*/false);
-  filterNonConflictingPreviousDecls(Context, NewTD, Previous);
+  filterNonConflictingPreviousTypedefDecls(Context, NewTD, Previous);
   if (!Previous.empty()) {
     Redeclaration = true;
     MergeTypedefNameDecl(NewTD, Previous);

clang/test/Modules/Inputs/merge-typedefs/a1.h

@@ -0,0 +1,8 @@
+#ifndef A1_H
+#define A1_H
+namespace llvm {
+class MachineBasicBlock;
+template <class NodeT> class DomTreeNodeBase;
+typedef DomTreeNodeBase<MachineBasicBlock> MachineDomTreeNode;
+}
+#endif

clang/test/Modules/Inputs/merge-typedefs/a2.h

@@ -0,0 +1,3 @@
+#ifndef A2_H
+#define A2_H
+#endif

clang/test/Modules/Inputs/merge-typedefs/b1.h

@@ -0,0 +1,9 @@
+#ifndef B1_H
+#define B1_H
+#include "a2.h"
+namespace llvm {
+class MachineBasicBlock;
+template <class NodeT> class DomTreeNodeBase;
+typedef DomTreeNodeBase<MachineBasicBlock> MachineDomTreeNode;
+}
+#endif

clang/test/Modules/Inputs/merge-typedefs/b2.h

@@ -0,0 +1,3 @@
+#ifndef B2_H
+#define B2_H
+#endif

clang/test/Modules/Inputs/merge-typedefs/module.modulemap

@@ -0,0 +1,9 @@
+module A {
+  module A1 { header "a1.h" export * }
+  module A2 { header "a2.h" export * }
+}
+
+module B {
+  module B1 { header "b1.h" export * }
+  module B2 { header "b2.h" export * }
+}

clang/test/Modules/merge-typedefs.cpp

@@ -0,0 +1,9 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -x c++ -I%S/Inputs/merge-typedefs -verify %s
+// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t -x c++ -I%S/Inputs/merge-typedefs -verify %s
+
+#include "b2.h"
+#include "a1.h"
+
+// expected-no-diagnostics
+llvm::MachineDomTreeNode *p;