r240700 - [modules] Properly merge visibility of class definitions that got merged while

Richard Smith richard-llvm at metafoo.co.uk
Thu Jun 25 14:42:33 PDT 2015


Author: rsmith
Date: Thu Jun 25 16:42:33 2015
New Revision: 240700

URL: http://llvm.org/viewvc/llvm-project?rev=240700&view=rev
Log:
[modules] Properly merge visibility of class definitions that got merged while
parsing then merged again when the module was loaded.

Added:
    cfe/trunk/test/Modules/Inputs/merge-class-definition-visibility/e.h
Modified:
    cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
    cfe/trunk/test/Modules/Inputs/merge-class-definition-visibility/b.h
    cfe/trunk/test/Modules/Inputs/merge-class-definition-visibility/d.h
    cfe/trunk/test/Modules/Inputs/merge-class-definition-visibility/modmap
    cfe/trunk/test/Modules/merge-class-definition-visibility.cpp

Modified: cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderDecl.cpp?rev=240700&r1=240699&r2=240700&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTReaderDecl.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReaderDecl.cpp Thu Jun 25 16:42:33 2015
@@ -3897,16 +3897,18 @@ void ASTDeclReader::UpdateDecl(Decl *D,
       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;
     }

Modified: cfe/trunk/test/Modules/Inputs/merge-class-definition-visibility/b.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/merge-class-definition-visibility/b.h?rev=240700&r1=240699&r2=240700&view=diff
==============================================================================
--- cfe/trunk/test/Modules/Inputs/merge-class-definition-visibility/b.h (original)
+++ cfe/trunk/test/Modules/Inputs/merge-class-definition-visibility/b.h Thu Jun 25 16:42:33 2015
@@ -1,2 +1,4 @@
 // Include definition of A into the same module as c.h
 #include "a.h"
+
+struct B {};

Modified: cfe/trunk/test/Modules/Inputs/merge-class-definition-visibility/d.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/merge-class-definition-visibility/d.h?rev=240700&r1=240699&r2=240700&view=diff
==============================================================================
--- cfe/trunk/test/Modules/Inputs/merge-class-definition-visibility/d.h (original)
+++ cfe/trunk/test/Modules/Inputs/merge-class-definition-visibility/d.h Thu Jun 25 16:42:33 2015
@@ -1 +1 @@
-#include "a.h"
+struct B {};

Added: cfe/trunk/test/Modules/Inputs/merge-class-definition-visibility/e.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/merge-class-definition-visibility/e.h?rev=240700&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/merge-class-definition-visibility/e.h (added)
+++ cfe/trunk/test/Modules/Inputs/merge-class-definition-visibility/e.h Thu Jun 25 16:42:33 2015
@@ -0,0 +1,3 @@
+#include "a.h"
+
+struct B {};

Modified: cfe/trunk/test/Modules/Inputs/merge-class-definition-visibility/modmap
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/merge-class-definition-visibility/modmap?rev=240700&r1=240699&r2=240700&view=diff
==============================================================================
--- cfe/trunk/test/Modules/Inputs/merge-class-definition-visibility/modmap (original)
+++ cfe/trunk/test/Modules/Inputs/merge-class-definition-visibility/modmap Thu Jun 25 16:42:33 2015
@@ -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" }
 }

Modified: cfe/trunk/test/Modules/merge-class-definition-visibility.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/merge-class-definition-visibility.cpp?rev=240700&r1=240699&r2=240700&view=diff
==============================================================================
--- cfe/trunk/test/Modules/merge-class-definition-visibility.cpp (original)
+++ cfe/trunk/test/Modules/merge-class-definition-visibility.cpp Thu Jun 25 16:42:33 2015
@@ -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;





More information about the cfe-commits mailing list