When performing name lookup for a redeclaration, ignore module

visibility restrictions. This ensures that all declarations of the
same entity end up in the same redeclaration chain, even if some of
those declarations aren't visible. While this may seem unfortunate to
some---why can't two C modules have different functions named
'f'?---it's an acknowedgment that a module does not introduce a new
"namespace" of names.

As part of this, stop merging the 'module-private' bit from previous
declarations to later declarations, because we want each declaration
in a module to stand on its own because this can effect, for example,
submodule visibility.

Note that this notion of names that are invisible to normal name
lookup but are available for redeclaration lookups is how we should
implement friend declarations and extern declarations within local
function scopes. I'm not tackling that problem now.

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

diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index c0676dd3e1dc2b81cc3f67448e47f528acf4b8c4..7c2e1f879767afe6502c45a2898fedecad675eaf 100644 (file)
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -5154,8 +5154,6 @@ def note_related_result_type_inferred : Note<
 }
 
 let CategoryName = "Modules Issue" in {
-def err_module_private_follows_public : Error<
-  "__module_private__ declaration of %0 follows public declaration">;
 def err_module_private_specialization : Error<
   "%select{template|partial|member}0 specialization cannot be "
   "declared __module_private__">;

diff --git a/include/clang/Sema/Lookup.h b/include/clang/Sema/Lookup.h
index 9f8a239d9f22b63788bc51f9d9243c3536a1ccb0..1b991cb54b96d9c214e51e297ca8814d86101370 100644 (file)
--- a/include/clang/Sema/Lookup.h
+++ b/include/clang/Sema/Lookup.h
@@ -286,7 +286,7 @@ public:
     if (!D->isInIdentifierNamespace(IDNS))
       return 0;
     
-    if (isVisible(D))
+    if (Redecl == Sema::ForRedeclaration || isVisible(D))
       return D;
     
     return getAcceptableDeclSlow(D);

diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h
index fd87d6b2de73ed94731ee5a31a52cc28dac62e03..9b244cdc84f55545f2017d0dccb51f02fe5633cb 100644 (file)
--- a/include/clang/Sema/Sema.h
+++ b/include/clang/Sema/Sema.h
@@ -1119,12 +1119,6 @@ public:
   /// \param Path The module access path.
   DeclResult ActOnModuleImport(SourceLocation ImportLoc, ModuleIdPath Path);
 
-  /// \brief Diagnose that \p New is a module-private redeclaration of
-  /// \p Old.
-  void diagnoseModulePrivateRedeclaration(NamedDecl *New, NamedDecl *Old,
-                                          SourceLocation ModulePrivateKeyword
-                                            = SourceLocation());
-
   /// \brief Retrieve a suitable printing policy.
   PrintingPolicy getPrintingPolicy() const;
 

diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp
index c31850313ad47ab68ed4e7e9362dabbb0d8201ab..ebcdcf58d047dab2be230b552bbb08260cab7737 100644 (file)
--- a/lib/Sema/SemaDecl.cpp
+++ b/lib/Sema/SemaDecl.cpp
@@ -1456,12 +1456,6 @@ void Sema::MergeTypedefNameDecl(TypedefNameDecl *New, LookupResult &OldDecls) {
   if (TypedefNameDecl *Typedef = dyn_cast<TypedefNameDecl>(Old))
     New->setPreviousDeclaration(Typedef);
 
-  // __module_private__ is propagated to later declarations.
-  if (Old->isModulePrivate())
-    New->setModulePrivate();
-  else if (New->isModulePrivate())
-    diagnoseModulePrivateRedeclaration(New, Old);
-           
   if (getLangOptions().MicrosoftExt)
     return;
 
@@ -2047,12 +2041,6 @@ bool Sema::MergeCompatibleFunctionDecls(FunctionDecl *New, FunctionDecl *Old) {
   if (Old->isPure())
     New->setPure();
 
-  // __module_private__ is propagated to later declarations.
-  if (Old->isModulePrivate())
-    New->setModulePrivate();
-  else if (New->isModulePrivate())
-    diagnoseModulePrivateRedeclaration(New, Old);
-
   // Merge attributes from the parameters.  These can mismatch with K&R
   // declarations.
   if (New->getNumParams() == Old->getNumParams())
@@ -2237,12 +2225,6 @@ void Sema::MergeVarDecl(VarDecl *New, LookupResult &Previous) {
     return New->setInvalidDecl();
   }
 
-  // __module_private__ is propagated to later declarations.
-  if (Old->isModulePrivate())
-    New->setModulePrivate();
-  else if (New->isModulePrivate())
-    diagnoseModulePrivateRedeclaration(New, Old);
-
   // Variables with external linkage are analyzed in FinalizeDeclaratorGroup.
 
   // FIXME: The test for external storage here seems wrong? We still
@@ -5627,9 +5609,6 @@ bool Sema::CheckFunctionDeclaration(Scope *S, FunctionDecl *NewFD,
           assert(OldTemplateDecl->isMemberSpecialization());
         }
         
-        if (OldTemplateDecl->isModulePrivate())
-          NewTemplateDecl->setModulePrivate();
-        
       } else {
         if (isa<CXXMethodDecl>(NewFD)) // Set access for out-of-line definitions
           NewFD->setAccess(OldDecl->getAccess());
@@ -8212,19 +8191,14 @@ CreateNewDecl:
     AddMsStructLayoutForRecord(RD);
   }
 
-  if (PrevDecl && PrevDecl->isModulePrivate())
-    New->setModulePrivate();
-  else if (ModulePrivateLoc.isValid()) {
+  if (ModulePrivateLoc.isValid()) {
     if (isExplicitSpecialization)
       Diag(New->getLocation(), diag::err_module_private_specialization)
         << 2
         << FixItHint::CreateRemoval(ModulePrivateLoc);
-    else if (PrevDecl && !PrevDecl->isModulePrivate())
-      diagnoseModulePrivateRedeclaration(New, PrevDecl, ModulePrivateLoc);
     // __module_private__ does not apply to local classes. However, we only
     // diagnose this as an error when the declaration specifiers are
     // freestanding. Here, we just ignore the __module_private__.
-    // foobar
     else if (!SearchDC->isFunctionOrMethod())
       New->setModulePrivate();
   }
@@ -9969,20 +9943,6 @@ DeclResult Sema::ActOnModuleImport(SourceLocation ImportLoc, ModuleIdPath Path)
   return Import;
 }
 
-void 
-Sema::diagnoseModulePrivateRedeclaration(NamedDecl *New, NamedDecl *Old,
-                                         SourceLocation ModulePrivateKeyword) {
-  assert(!Old->isModulePrivate() && "Old is module-private!");
-  
-  Diag(New->getLocation(), diag::err_module_private_follows_public)
-    << New->getDeclName() << SourceRange(ModulePrivateKeyword);
-  Diag(Old->getLocation(), diag::note_previous_declaration)
-    << Old->getDeclName();
-  
-  // Drop the __module_private__ from the new declaration, since it's invalid.
-  New->setModulePrivate(false);
-}
-
 void Sema::ActOnPragmaWeakID(IdentifierInfo* Name,
                              SourceLocation PragmaLoc,
                              SourceLocation NameLoc) {

diff --git a/lib/Sema/SemaLookup.cpp b/lib/Sema/SemaLookup.cpp
index bc1cb3fca63bd9db1b4f6493aa52c11f288fe65d..a5fc682d18c96052a30758704f03bff572b6939e 100644 (file)
--- a/lib/Sema/SemaLookup.cpp
+++ b/lib/Sema/SemaLookup.cpp
@@ -1169,7 +1169,7 @@ bool Sema::LookupName(LookupResult &R, Scope *S, bool AllowBuiltinCreation) {
         
         // If this declaration is module-private and it came from an AST
         // file, we can't see it.
-        NamedDecl *D = getVisibleDecl(*I);
+        NamedDecl *D = R.isForRedeclaration()? *I : getVisibleDecl(*I);
         if (!D)
           continue;
                 

diff --git a/lib/Sema/SemaTemplate.cpp b/lib/Sema/SemaTemplate.cpp
index 8ec4c195f919428056753d4c235281b5ab1ea631..d0c0f0bf2d12e15f5ca854fcff39c1e64041b4a2 100644 (file)
--- a/lib/Sema/SemaTemplate.cpp
+++ b/lib/Sema/SemaTemplate.cpp
@@ -1013,15 +1013,8 @@ Sema::CheckClassTemplate(Scope *S, unsigned TagSpec, TagUseKind TUK,
                                 NewClass, PrevClassTemplate);
   NewClass->setDescribedClassTemplate(NewTemplate);
   
-  if (PrevClassTemplate && PrevClassTemplate->isModulePrivate()) {
+  if (ModulePrivateLoc.isValid())
     NewTemplate->setModulePrivate();
-  } else if (ModulePrivateLoc.isValid()) {
-    if (PrevClassTemplate && !PrevClassTemplate->isModulePrivate())
-      diagnoseModulePrivateRedeclaration(NewTemplate, PrevClassTemplate,
-                                         ModulePrivateLoc);
-    else
-      NewTemplate->setModulePrivate();
-  }
   
   // Build the type for the class template declaration now.
   QualType T = NewTemplate->getInjectedClassNameSpecialization();

diff --git a/test/Modules/Inputs/module.map b/test/Modules/Inputs/module.map
index b101c01567d694db6282262e44ca93683b4e6ab2..13a44eb6a76df7ed4b8b37d1e7accc90bacd9b72 100644 (file)
--- a/test/Modules/Inputs/module.map
+++ b/test/Modules/Inputs/module.map
@@ -44,7 +44,8 @@ module decldef {
 }
 
 module redecl_merge_top { 
-  header "redecl-merge-top.h" 
+  header "redecl-merge-top.h"
+  explicit module Explicit { header "redecl-merge-top-explicit.h" }
 }
 module redecl_merge_left { 
   header "redecl-merge-left.h" 

diff --git a/test/Modules/Inputs/module_private_left.h b/test/Modules/Inputs/module_private_left.h
index 617a7abf3d98fb1bf087726c7c5585f88e284a99..ff33999d0fbf43d9f69e708360ae871410b0ccac 100644 (file)
--- a/test/Modules/Inputs/module_private_left.h
+++ b/test/Modules/Inputs/module_private_left.h
@@ -1,6 +1,6 @@
 __module_private__ struct HiddenStruct;
 
-struct HiddenStruct {
+__module_private__ struct HiddenStruct {
 };
 
 
@@ -10,17 +10,17 @@ template<typename T>
 __module_private__ void f1(T*);
 
 template<typename T>
-void f1(T*);
+__module_private__ void f1(T*);
 
 template<typename T>
 __module_private__ class vector;
 
 template<typename T>
-class vector {
+__module_private__ class vector {
 };
 
 vector<float> vec_float;
 
 typedef __module_private__ int Integer;
-typedef int Integer;
+typedef __module_private__ int Integer;
 

diff --git a/test/Modules/Inputs/module_private_right.h b/test/Modules/Inputs/module_private_right.h
index e7c1bb822164567781bbd2b61e76b4eca86f68d3..53efe25d0889894bb40859456db292748ae70354 100644 (file)
--- a/test/Modules/Inputs/module_private_right.h
+++ b/test/Modules/Inputs/module_private_right.h
@@ -1,5 +1,5 @@
 __module_private__ double &f0(double);
-double &f0(double);
+__module_private__ double &f0(double);
 
 __module_private__ int hidden_var;
 

diff --git a/test/Modules/Inputs/redecl-merge-left.h b/test/Modules/Inputs/redecl-merge-left.h
index 8e2fa53ae1313492e7355758e2fcd8d718ad5725..4e009591de6869d18d08a1811d9bf1f2268b1413 100644 (file)
--- a/test/Modules/Inputs/redecl-merge-left.h
+++ b/test/Modules/Inputs/redecl-merge-left.h
@@ -10,6 +10,12 @@ __import_module__ redecl_merge_top;
 
 @class A;
 
+@class Explicit;
+
+int *explicit_func(void);
+
+struct explicit_struct;
+
 #ifdef __cplusplus
 template<typename T> class Vector;
 

diff --git a/test/Modules/Inputs/redecl-merge-right.h b/test/Modules/Inputs/redecl-merge-right.h
index 6d6973471145224e8f278772db411fe40b86898b..f0bfd35746f2611aa23e8d566e646fd26600336b 100644 (file)
--- a/test/Modules/Inputs/redecl-merge-right.h
+++ b/test/Modules/Inputs/redecl-merge-right.h
@@ -9,6 +9,12 @@ __import_module__ redecl_merge_top;
 
 @class B;
 
+@class Explicit;
+
+int *explicit_func(void);
+
+struct explicit_struct;
+
 #ifdef __cplusplus
 template<typename T> class Vector { 
 public:

diff --git a/test/Modules/Inputs/redecl-merge-top-explicit.h b/test/Modules/Inputs/redecl-merge-top-explicit.h
new file mode 100644 (file)
index 0000000..c8f5158
--- /dev/null
+++ b/test/Modules/Inputs/redecl-merge-top-explicit.h
@@ -0,0 +1,5 @@
+@class Explicit;
+
+int *explicit_func(void);
+
+struct explicit_struct { int member; };

diff --git a/test/Modules/module-private.cpp b/test/Modules/module-private.cpp
index eb932a8eb850fd1a8158d67c6292715b26e1023e..bdfa2682ce1e02381c01dbc1a5e97caef6decc23 100644 (file)
--- a/test/Modules/module-private.cpp
+++ b/test/Modules/module-private.cpp
@@ -31,28 +31,28 @@ int test_broken() {
 
 // Check for private redeclarations of public entities.
 template<typename T>
-class public_class_template; // expected-note{{previous declaration is here}}
+class public_class_template;
 
 template<typename T>
-__module_private__ class public_class_template; // expected-error{{__module_private__ declaration of 'public_class_template' follows public declaration}}
+__module_private__ class public_class_template;
 
 
-typedef int public_typedef; // expected-note{{previous declaration is here}}
-typedef __module_private__ int public_typedef; // expected-error{{__module_private__ declaration of 'public_typedef' follows public declaration}}
+typedef int public_typedef;
+typedef __module_private__ int public_typedef;
 
-extern int public_var; // expected-note{{previous declaration is here}}
-extern __module_private__ int public_var; // expected-error{{__module_private__ declaration of 'public_var' follows public declaration}}
+extern int public_var;
+extern __module_private__ int public_var;
 
-void public_func(); // expected-note{{previous declaration is here}}
-__module_private__ void public_func(); // expected-error{{__module_private__ declaration of 'public_func' follows public declaration}}
+void public_func();
+__module_private__ void public_func();
 
 template<typename T>
-void public_func_template(); // expected-note{{previous declaration is here}}
+void public_func_template();
 template<typename T>
-__module_private__ void public_func_template(); // expected-error{{__module_private__ declaration of 'public_func_template' follows public declaration}}
+__module_private__ void public_func_template();
 
-struct public_struct; // expected-note{{previous declaration is here}}
-__module_private__ struct public_struct; // expected-error{{__module_private__ declaration of 'public_struct' follows public declaration}}
+struct public_struct;
+__module_private__ struct public_struct;
 
 // Check for attempts to make specializations private
 template<> __module_private__ void public_func_template<int>(); // expected-error{{template specialization cannot be declared __module_private__}}

diff --git a/test/Modules/redecl-merge.m b/test/Modules/redecl-merge.m
index 6497268c3840acda1112d89a46337297a5255d17..7021d14f6984acc18982e2827b674e27b6dbf016 100644 (file)
--- a/test/Modules/redecl-merge.m
+++ b/test/Modules/redecl-merge.m
@@ -20,6 +20,18 @@ B *f1() {
 
 @class B;
 
+// Test redeclarations of entities in explicit submodules, to make
+// sure we're maintaining the declaration chains even when normal name
+// lookup can't see what we're looking for.
+void testExplicit() {
+  Explicit *e;
+  int *(*fp)(void) = &explicit_func;
+  int *ip = explicit_func();
+
+  // FIXME: Should complain about definition not having been imported.
+  struct explicit_struct es = { 0 };
+}
+
 __import_module__ redecl_merge_bottom;
 
 @implementation B