r233420 - [modules] When merging class definitions, make the retained definition visible
Richard Smith
richard-llvm at metafoo.co.uk
Fri Mar 27 14:16:39 PDT 2015
Author: rsmith
Date: Fri Mar 27 16:16:39 2015
New Revision: 233420
URL: http://llvm.org/viewvc/llvm-project?rev=233420&view=rev
Log:
[modules] When merging class definitions, make the retained definition visible
if the merged definition is visible, and perform lookups into all merged copies
of the definition (not just for special members) so that we can complete the
redecl chains for members of the class.
Modified:
cfe/trunk/lib/Serialization/ASTReader.cpp
cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
cfe/trunk/test/Modules/Inputs/redecl-add-after-load-decls.h
cfe/trunk/test/Modules/Inputs/submodules-merge-defs/module.modulemap
cfe/trunk/test/Modules/cxx-templates.cpp
cfe/trunk/test/Modules/redecl-add-after-load.cpp
cfe/trunk/test/Modules/submodules-merge-defs.cpp
Modified: cfe/trunk/lib/Serialization/ASTReader.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=233420&r1=233419&r2=233420&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTReader.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReader.cpp Fri Mar 27 16:16:39 2015
@@ -6730,20 +6730,14 @@ ASTReader::FindExternalVisibleDeclsByNam
// individually, because finding an entity in one of them doesn't imply that
// we can't find a different entity in another one.
if (isa<CXXRecordDecl>(DC)) {
- auto Kind = Name.getNameKind();
- if (Kind == DeclarationName::CXXConstructorName ||
- Kind == DeclarationName::CXXDestructorName ||
- (Kind == DeclarationName::CXXOperatorName &&
- Name.getCXXOverloadedOperator() == OO_Equal)) {
- auto Merged = MergedLookups.find(DC);
- if (Merged != MergedLookups.end()) {
- for (unsigned I = 0; I != Merged->second.size(); ++I) {
- const DeclContext *Context = Merged->second[I];
- LookUpInContexts(Context);
- // We might have just added some more merged lookups. If so, our
- // iterator is now invalid, so grab a fresh one before continuing.
- Merged = MergedLookups.find(DC);
- }
+ auto Merged = MergedLookups.find(DC);
+ if (Merged != MergedLookups.end()) {
+ for (unsigned I = 0; I != Merged->second.size(); ++I) {
+ const DeclContext *Context = Merged->second[I];
+ LookUpInContexts(Context);
+ // We might have just added some more merged lookups. If so, our
+ // iterator is now invalid, so grab a fresh one before continuing.
+ Merged = MergedLookups.find(DC);
}
}
}
Modified: cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderDecl.cpp?rev=233420&r1=233419&r2=233420&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTReaderDecl.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReaderDecl.cpp Fri Mar 27 16:16:39 2015
@@ -1385,6 +1385,29 @@ void ASTDeclReader::MergeDefinitionData(
"merging class definition into non-definition");
auto &DD = *D->DefinitionData.getNotUpdated();
+ // If the new definition has new special members, let the name lookup
+ // code know that it needs to look in the new definition too.
+ //
+ // FIXME: We only need to do this if the merged definition declares members
+ // that this definition did not declare, or if it defines members that this
+ // definition did not define.
+ if (DD.Definition != MergeDD.Definition) {
+ Reader.MergedLookups[DD.Definition].push_back(MergeDD.Definition);
+ DD.Definition->setHasExternalVisibleStorage();
+
+ if (DD.Definition->isHidden()) {
+ // If MergeDD is visible or becomes visible, make the definition visible.
+ if (!MergeDD.Definition->isHidden())
+ DD.Definition->Hidden = false;
+ else {
+ auto SubmoduleID = MergeDD.Definition->getOwningModuleID();
+ assert(SubmoduleID && "hidden definition in no module");
+ Reader.HiddenNamesMap[Reader.getSubmodule(SubmoduleID)]
+ .HiddenDecls.push_back(DD.Definition);
+ }
+ }
+ }
+
auto PFDI = Reader.PendingFakeDefinitionData.find(&DD);
if (PFDI != Reader.PendingFakeDefinitionData.end() &&
PFDI->second == ASTReader::PendingFakeDefinitionKind::Fake) {
@@ -1401,17 +1424,6 @@ void ASTDeclReader::MergeDefinitionData(
return;
}
- // If the new definition has new special members, let the name lookup
- // code know that it needs to look in the new definition too.
- //
- // FIXME: We only need to do this if the merged definition declares members
- // that this definition did not declare, or if it defines members that this
- // definition did not define.
- if (MergeDD.DeclaredSpecialMembers && DD.Definition != MergeDD.Definition) {
- Reader.MergedLookups[DD.Definition].push_back(MergeDD.Definition);
- DD.Definition->setHasExternalVisibleStorage();
- }
-
// FIXME: Move this out into a .def file?
bool DetectedOdrViolation = false;
#define OR_FIELD(Field) DD.Field |= MergeDD.Field;
Modified: cfe/trunk/test/Modules/Inputs/redecl-add-after-load-decls.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/redecl-add-after-load-decls.h?rev=233420&r1=233419&r2=233420&view=diff
==============================================================================
--- cfe/trunk/test/Modules/Inputs/redecl-add-after-load-decls.h (original)
+++ cfe/trunk/test/Modules/Inputs/redecl-add-after-load-decls.h Fri Mar 27 16:16:39 2015
@@ -16,9 +16,9 @@ typedef C::A CB;
constexpr int C_test(bool b) { return b ? C::variable : C::function(); }
struct D {
- struct A; // expected-note {{forward}}
+ struct A;
static const int variable;
- static constexpr int function(); // expected-note {{here}}
+ static constexpr int function();
};
typedef D::A DB;
-constexpr int D_test(bool b) { return b ? D::variable : D::function(); } // expected-note {{subexpression}} expected-note {{undefined}}
+constexpr int D_test(bool b) { return b ? D::variable : D::function(); }
Modified: cfe/trunk/test/Modules/Inputs/submodules-merge-defs/module.modulemap
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/submodules-merge-defs/module.modulemap?rev=233420&r1=233419&r2=233420&view=diff
==============================================================================
--- cfe/trunk/test/Modules/Inputs/submodules-merge-defs/module.modulemap (original)
+++ cfe/trunk/test/Modules/Inputs/submodules-merge-defs/module.modulemap Fri Mar 27 16:16:39 2015
@@ -9,3 +9,8 @@ module "redef" {
// Do not re-export stuff.use
use "stuff"
}
+
+module "merged-defs" {
+ header "merged-defs.h"
+ use "stuff"
+}
Modified: cfe/trunk/test/Modules/cxx-templates.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/cxx-templates.cpp?rev=233420&r1=233419&r2=233420&view=diff
==============================================================================
--- cfe/trunk/test/Modules/cxx-templates.cpp (original)
+++ cfe/trunk/test/Modules/cxx-templates.cpp Fri Mar 27 16:16:39 2015
@@ -125,10 +125,7 @@ void g() {
static_assert(Outer<int>::Inner<int>::f() == 1, "");
static_assert(Outer<int>::Inner<int>::g() == 2, "");
-// FIXME: We're too lazy in merging class definitions to find the definition
-// of this function.
-static_assert(MergeTemplateDefinitions<int>::f() == 1, ""); // expected-error {{constant expression}} expected-note {{undefined}}
-// expected-note at cxx-templates-c.h:10 {{here}}
+static_assert(MergeTemplateDefinitions<int>::f() == 1, "");
static_assert(MergeTemplateDefinitions<int>::g() == 2, "");
RedeclaredAsFriend<int> raf1;
Modified: cfe/trunk/test/Modules/redecl-add-after-load.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/redecl-add-after-load.cpp?rev=233420&r1=233419&r2=233420&view=diff
==============================================================================
--- cfe/trunk/test/Modules/redecl-add-after-load.cpp (original)
+++ cfe/trunk/test/Modules/redecl-add-after-load.cpp Fri Mar 27 16:16:39 2015
@@ -2,8 +2,9 @@
// RUN: %clang_cc1 -x objective-c++ -fmodules -fno-modules-error-recovery -fmodules-cache-path=%t -I %S/Inputs %s -verify -std=c++11
// RUN: %clang_cc1 -x objective-c++ -fmodules -fno-modules-error-recovery -fmodules-cache-path=%t -I %S/Inputs %s -verify -std=c++11 -DIMPORT_DECLS
-#ifdef IMPORT_DECLS
// expected-no-diagnostics
+
+#ifdef IMPORT_DECLS
@import redecl_add_after_load_decls;
#else
typedef struct A B;
@@ -24,12 +25,12 @@ typedef C::A CB;
constexpr int C_test(bool b) { return b ? C::variable : C::function(); }
struct D {
- struct A; // expected-note {{forward}}
+ struct A;
static const int variable;
- static constexpr int function(); // expected-note {{here}}
+ static constexpr int function();
};
typedef D::A DB;
-constexpr int D_test(bool b) { return b ? D::variable : D::function(); } // expected-note {{undefined}}
+constexpr int D_test(bool b) { return b ? D::variable : D::function(); }
#endif
@import redecl_add_after_load;
@@ -46,14 +47,6 @@ CB struct_struct_test;
constexpr int struct_variable_test = C_test(true);
constexpr int struct_function_test = C_test(false);
-// FIXME: We should accept this, but we're currently too lazy when merging class
-// definitions to determine that the definitions in redecl_add_after_load are
-// definitions of these entities.
DB merged_struct_struct_test;
constexpr int merged_struct_variable_test = D_test(true);
constexpr int merged_struct_function_test = D_test(false);
-#ifndef IMPORT_DECLS
-// expected-error at -4 {{incomplete}}
-// @-4: definition of D::variable must be emitted, so it gets imported eagerly
-// expected-error at -4 {{constant}} expected-note at -4 {{in call to}}
-#endif
Modified: cfe/trunk/test/Modules/submodules-merge-defs.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/submodules-merge-defs.cpp?rev=233420&r1=233419&r2=233420&view=diff
==============================================================================
--- cfe/trunk/test/Modules/submodules-merge-defs.cpp (original)
+++ cfe/trunk/test/Modules/submodules-merge-defs.cpp Fri Mar 27 16:16:39 2015
@@ -1,4 +1,5 @@
// RUN: rm -rf %t
+// RUN: %clang_cc1 -x c++ -std=c++11 -fmodules-cache-path=%t -fmodules -I %S/Inputs/submodules-merge-defs %s -verify -fno-modules-error-recovery -DTEXTUAL
// RUN: %clang_cc1 -x c++ -std=c++11 -fmodules-cache-path=%t -fmodules -I %S/Inputs/submodules-merge-defs %s -verify -fno-modules-error-recovery
// Trigger import of definitions, but don't make them visible.
@@ -27,7 +28,11 @@ D::X pre_dx; // expected-error +{{must b
int pre_use_dx = use_dx(pre_dx);
// Make definitions from second module visible.
+#ifdef TEXTUAL
#include "import-and-redefine.h"
+#else
+#include "merged-defs.h"
+#endif
A post_a;
int post_use_a = use_a(post_a);
More information about the cfe-commits
mailing list