On 30/10/17 23:40, Richard Smith wrote:
On 30 October 2017 at 15:12, Vassil Vassilev via cfe-commits
<cfe-commits@lists.llvm.org <mailto:cfe-commits@lists.llvm.org>> wrote:
On 11/10/17 03:19, Richard Smith via cfe-commits wrote:
Author: rsmith
Date: Tue Oct 10 18:19:11 2017
New Revision: 315402
URL: http://llvm.org/viewvc/llvm-project?rev=315402&view=rev
<http://llvm.org/viewvc/llvm-project?rev=315402&view=rev>
Log:
[modules] Only take visible using-directives into account
during name lookup.
Added:
cfe/trunk/test/Modules/using-directive.cpp
Modified:
cfe/trunk/lib/Sema/SemaLookup.cpp
Modified: cfe/trunk/lib/Sema/SemaLookup.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaLookup.cpp?rev=315402&r1=315401&r2=315402&view=diff
<http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaLookup.cpp?rev=315402&r1=315401&r2=315402&view=diff>
==============================================================================
--- cfe/trunk/lib/Sema/SemaLookup.cpp (original)
+++ cfe/trunk/lib/Sema/SemaLookup.cpp Tue Oct 10 18:19:11 2017
@@ -88,13 +88,15 @@ namespace {
/// A collection of using directives, as used by C++
unqualified
/// lookup.
class UnqualUsingDirectiveSet {
+ Sema &SemaRef;
+
typedef SmallVector<UnqualUsingEntry, 8> ListTy;
ListTy list;
llvm::SmallPtrSet<DeclContext*, 8> visited;
public:
- UnqualUsingDirectiveSet() {}
+ UnqualUsingDirectiveSet(Sema &SemaRef) : SemaRef(SemaRef) {}
void visitScopeChain(Scope *S, Scope
*InnermostFileScope) {
// C++ [namespace.udir]p1:
@@ -113,7 +115,8 @@ namespace {
visit(Ctx, Ctx);
} else if (!Ctx || Ctx->isFunctionOrMethod()) {
for (auto *I : S->using_directives())
- visit(I, InnermostFileDC);
+ if (SemaRef.isVisible(I))
+ visit(I, InnermostFileDC);
}
}
}
@@ -152,7 +155,7 @@ namespace {
while (true) {
for (auto UD : DC->using_directives()) {
DeclContext *NS = UD->getNominatedNamespace();
- if (visited.insert(NS).second) {
+ if (visited.insert(NS).second &&
SemaRef.isVisible(UD)) {
It looks like this change breaks examples such as:
// RUN: rm -fr %t
// RUN %clang_cc1 -fmodules -fmodules-local-submodule-visibility
-fmodules-cache-path=%t -verify %s
// expected-no-diagnostics
#pragma clang module build M
module M { module TDFNodes {} module TDFInterface {} }
#pragma clang module contents
// TDFNodes
#pragma clang module begin M.TDFNodes
namespace Detail {
namespace TDF {
class TLoopManager {};
}
}
namespace Internal {
namespace TDF {
using namespace Detail::TDF;
}
}
#pragma clang module end
// TDFInterface
#pragma clang module begin M.TDFInterface
#pragma clang module import M.TDFNodes
namespace Internal {
namespace TDF {
using namespace Detail::TDF;
}
}
#pragma clang module end
#pragma clang module endbuild
#pragma clang module import M.TDFNodes
namespace Internal {
namespace TDF {
TLoopManager * use;
}
}
I suspect that it triggers a preexisting bug in failing to make
M.TDFNodes visible...
This code in NamedDecl::declarationReplaces is wrong, we even have a
FIXME for the bug:
// UsingDirectiveDecl's are not really NamedDecl's, and all have
same name.
// They can be replaced if they nominate the same namespace.
// FIXME: Is this true even if they have different module visibility?
if (auto *UD = dyn_cast<UsingDirectiveDecl>(this))
return UD->getNominatedNamespace()->getOriginalNamespace() ==
cast<UsingDirectiveDecl>(OldD)->getNominatedNamespace()
->getOriginalNamespace();
Fixed in r316965 by deleting the above code :)
You ninjaed me I just understood that the getOriginalNamespace was wrong
and hunting that down :D
Thanks for the quick fix!
This might introduce a performance regression in the presence of large
numbers of duplicated using-directives. We can rectify that, if
needed, by making using-directives redeclarable, but I'm not expecting
that to be a problem outside of pathological cases.
addUsingDirective(UD, EffectiveDC);
queue.push_back(NS);
}
@@ -1085,7 +1088,7 @@ bool Sema::CppLookupName(LookupResult &R
// }
// }
//
- UnqualUsingDirectiveSet UDirs;
+ UnqualUsingDirectiveSet UDirs(*this);
bool VisitedUsingDirectives = false;
bool LeftStartingScope = false;
DeclContext *OutsideOfTemplateParamDC = nullptr;
@@ -1868,22 +1871,19 @@ static bool LookupQualifiedNameInUsingDi
DeclContext *StartDC) {
assert(StartDC->isFileContext() && "start context is not a
file context");
- DeclContext::udir_range UsingDirectives =
StartDC->using_directives();
- if (UsingDirectives.begin() == UsingDirectives.end())
return false;
+ // We have not yet looked into these namespaces, much less
added
+ // their "using-children" to the queue.
+ SmallVector<NamespaceDecl*, 8> Queue;
// We have at least added all these contexts to the queue.
llvm::SmallPtrSet<DeclContext*, 8> Visited;
Visited.insert(StartDC);
- // We have not yet looked into these namespaces, much
less added
- // their "using-children" to the queue.
- SmallVector<NamespaceDecl*, 8> Queue;
-
// We have already looked into the initial namespace; seed
the queue
// with its using-children.
- for (auto *I : UsingDirectives) {
+ for (auto *I : StartDC->using_directives()) {
NamespaceDecl *ND =
I->getNominatedNamespace()->getOriginalNamespace();
- if (Visited.insert(ND).second)
+ if (Visited.insert(ND).second && S.isVisible(I))
Queue.push_back(ND);
}
@@ -1931,7 +1931,7 @@ static bool LookupQualifiedNameInUsingDi
for (auto I : ND->using_directives()) {
NamespaceDecl *Nom = I->getNominatedNamespace();
- if (Visited.insert(Nom).second)
+ if (Visited.insert(Nom).second && S.isVisible(I))
Queue.push_back(Nom);
}
}
@@ -3540,6 +3540,8 @@ static void LookupVisibleDecls(DeclConte
if (QualifiedNameLookup) {
ShadowContextRAII Shadow(Visited);
for (auto I : Ctx->using_directives()) {
+ if (!Result.getSema().isVisible(I))
+ continue;
LookupVisibleDecls(I->getNominatedNamespace(), Result,
QualifiedNameLookup, InBaseClass,
Consumer, Visited,
IncludeDependentBases);
@@ -3746,7 +3748,7 @@ void Sema::LookupVisibleDecls(Scope *S,
// Determine the set of using directives available during
// unqualified name lookup.
Scope *Initial = S;
- UnqualUsingDirectiveSet UDirs;
+ UnqualUsingDirectiveSet UDirs(*this);
if (getLangOpts().CPlusPlus) {
// Find the first namespace or translation-unit scope.
while (S && !isNamespaceOrTranslationUnitScope(S))
Added: cfe/trunk/test/Modules/using-directive.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/using-directive.cpp?rev=315402&view=auto
<http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/using-directive.cpp?rev=315402&view=auto>
==============================================================================
--- cfe/trunk/test/Modules/using-directive.cpp (added)
+++ cfe/trunk/test/Modules/using-directive.cpp Tue Oct 10
18:19:11 2017
@@ -0,0 +1,62 @@
+// RUN: %clang_cc1 -fmodules
-fmodules-local-submodule-visibility
-fno-modules-error-recovery -fno-spell-checking -verify %s
+
+#pragma clang module build a
+module a { explicit module b {} explicit module c {} }
+#pragma clang module contents
+
+#pragma clang module begin a.b
+namespace b { int n; }
+#pragma clang module end
+
+#pragma clang module begin a.c
+#pragma clang module import a.b
+namespace c { using namespace b; }
+#pragma clang module end
+
+#pragma clang module begin a
+#pragma clang module import a.c
+using namespace c;
+#pragma clang module end
+
+#pragma clang module endbuild
+
+#pragma clang module import a.b
+void use1() {
+ (void)n; // expected-error {{use of undeclared identifier}}
+ (void)::n; // expected-error {{no member named 'n' in the
global namespace}}
+ (void)b::n;
+}
+namespace b {
+ void use1_in_b() { (void)n; }
+}
+namespace c {
+ void use1_in_c() { (void)n; } // expected-error {{use of
undeclared identifier}}
+}
+
+#pragma clang module import a.c
+void use2() {
+ (void)n; // expected-error {{use of undeclared identifier}}
+ (void)::n; // expected-error {{no member named 'n' in the
global namespace}}
+ (void)b::n;
+ (void)c::n;
+}
+namespace b {
+ void use2_in_b() { (void)n; }
+}
+namespace c {
+ void use2_in_c() { (void)n; }
+}
+
+#pragma clang module import a
+void use3() {
+ (void)n;
+ (void)::n;
+ (void)b::n;
+ (void)c::n;
+}
+namespace b {
+ void use3_in_b() { (void)n; }
+}
+namespace c {
+ void use3_in_c() { (void)n; }
+}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org <mailto:cfe-commits@lists.llvm.org>
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
<http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits>
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org <mailto:cfe-commits@lists.llvm.org>
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
<http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits>
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
