From 1e02a5aac82a41db5092998349f2fc4de459da19 Mon Sep 17 00:00:00 2001
From: Richard Smith <richard-llvm@metafoo.co.uk>
Date: Thu, 25 Jun 2015 21:42:33 +0000
Subject: [PATCH] [modules] Properly merge visibility of class definitions that
 got merged while parsing then merged again when the module was loaded.

llvm-svn: 240700
---
 clang/lib/Serialization/ASTReaderDecl.cpp                  | 12 +++++++-----
 .../Modules/Inputs/merge-class-definition-visibility/b.h   |  2 ++
 .../Modules/Inputs/merge-class-definition-visibility/d.h   |  2 +-
 .../Modules/Inputs/merge-class-definition-visibility/e.h   |  3 +++
 .../Inputs/merge-class-definition-visibility/modmap        |  3 ++-
 clang/test/Modules/merge-class-definition-visibility.cpp   | 14 +++++++++++---
 6 files changed, 26 insertions(+), 10 deletions(-)
 create mode 100644 clang/test/Modules/Inputs/merge-class-definition-visibility/e.h

diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp
index 00ebd3e..f27426b 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -3897,16 +3897,18 @@ void ASTDeclReader::UpdateDecl(Decl *D, ModuleFile &ModuleFile,
       if (Reader.getContext().getLangOpts().ModulesLocalVisibility) {
         // FIXME: This doesn't send the right notifications if there are
         // ASTMutationListeners other than an ASTWriter.
-        Reader.getContext().mergeDefinitionIntoModule(cast<NamedDecl>(D), Owner,
-                                                      /*NotifyListeners*/false);
-        Reader.PendingMergedDefinitionsToDeduplicate.insert(cast<NamedDecl>(D));
+        Reader.getContext().mergeDefinitionIntoModule(
+            cast<NamedDecl>(Exported), Owner,
+            /*NotifyListeners*/ false);
+        Reader.PendingMergedDefinitionsToDeduplicate.insert(
+            cast<NamedDecl>(Exported));
       } else if (Owner && Owner->NameVisibility != Module::AllVisible) {
         // If Owner is made visible at some later point, make this declaration
         // visible too.
-        Reader.HiddenNamesMap[Owner].push_back(D);
+        Reader.HiddenNamesMap[Owner].push_back(Exported);
       } else {
         // The declaration is now visible.
-        D->Hidden = false;
+        Exported->Hidden = false;
       }
       break;
     }
diff --git a/clang/test/Modules/Inputs/merge-class-definition-visibility/b.h b/clang/test/Modules/Inputs/merge-class-definition-visibility/b.h
index 2b8f5f8..03c0ad9 100644
--- a/clang/test/Modules/Inputs/merge-class-definition-visibility/b.h
+++ b/clang/test/Modules/Inputs/merge-class-definition-visibility/b.h
@@ -1,2 +1,4 @@
 // Include definition of A into the same module as c.h
 #include "a.h"
+
+struct B {};
diff --git a/clang/test/Modules/Inputs/merge-class-definition-visibility/d.h b/clang/test/Modules/Inputs/merge-class-definition-visibility/d.h
index 2243de1..c51edab 100644
--- a/clang/test/Modules/Inputs/merge-class-definition-visibility/d.h
+++ b/clang/test/Modules/Inputs/merge-class-definition-visibility/d.h
@@ -1 +1 @@
-#include "a.h"
+struct B {};
diff --git a/clang/test/Modules/Inputs/merge-class-definition-visibility/e.h b/clang/test/Modules/Inputs/merge-class-definition-visibility/e.h
new file mode 100644
index 0000000..f126b50
--- /dev/null
+++ b/clang/test/Modules/Inputs/merge-class-definition-visibility/e.h
@@ -0,0 +1,3 @@
+#include "a.h"
+
+struct B {};
diff --git a/clang/test/Modules/Inputs/merge-class-definition-visibility/modmap b/clang/test/Modules/Inputs/merge-class-definition-visibility/modmap
index 7d988fb..dcb6587 100644
--- a/clang/test/Modules/Inputs/merge-class-definition-visibility/modmap
+++ b/clang/test/Modules/Inputs/merge-class-definition-visibility/modmap
@@ -3,5 +3,6 @@ module Def1 {
   module C { header "c.h" }
 }
 module Def2 {
-  header "d.h"
+  module D { header "d.h" }
+  module E { header "e.h" }
 }
diff --git a/clang/test/Modules/merge-class-definition-visibility.cpp b/clang/test/Modules/merge-class-definition-visibility.cpp
index e8602c0..ac4c951 100644
--- a/clang/test/Modules/merge-class-definition-visibility.cpp
+++ b/clang/test/Modules/merge-class-definition-visibility.cpp
@@ -1,15 +1,23 @@
 // RUN: rm -rf %t
 // RUN: %clang_cc1 -fmodules -fmodule-map-file=%S/Inputs/merge-class-definition-visibility/modmap \
 // RUN:            -I%S/Inputs/merge-class-definition-visibility \
-// RUN:            -fmodules-cache-path=%t %s -verify
+// RUN:            -fmodules-cache-path=%t %s -verify \
+// RUN:            -fmodules-local-submodule-visibility
 // expected-no-diagnostics
 
 #include "c.h"
 template<typename T> struct X { T t; };
 typedef X<A> XA;
+struct B;
 
-#include "d.h"
-// Ensure that this triggers the import of the second definition from d.h,
+#include "e.h"
+// Ensure that this triggers the import of the second definition from e.h,
 // which is necessary to make the definition of A visible in the template
 // instantiation.
 XA xa;
+
+// Ensure that we make the definition of B visible. We made the parse-merged
+// definition from e.h visible, which makes the definition from d.h visible,
+// and that definition was merged into the canonical definition from b.h,
+// so that becomes visible, and we have a visible definition.
+B b;
-- 
2.7.4