<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<div class="moz-cite-prefix">On 30/10/17 23:40, Richard Smith wrote:<br>
</div>
<blockquote type="cite"
cite="mid:CAOfiQqkRvig2c8EZR_9-xbrW7i8u9JjXnFtJtSTLFu=U5nebJg@mail.gmail.com">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote">On 30 October 2017 at 15:12, Vassil
Vassilev via cfe-commits <span dir="ltr"><<a
href="mailto:cfe-commits@lists.llvm.org" target="_blank"
moz-do-not-send="true">cfe-commits@lists.llvm.org</a>></span>
wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px
0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">
<div class="gmail-HOEnZb">
<div class="gmail-h5">On 11/10/17 03:19, Richard Smith
via cfe-commits wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px
0px 0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">
Author: rsmith<br>
Date: Tue Oct 10 18:19:11 2017<br>
New Revision: 315402<br>
<br>
URL: <a
href="http://llvm.org/viewvc/llvm-project?rev=315402&view=rev"
rel="noreferrer" target="_blank"
moz-do-not-send="true">http://llvm.org/viewvc/llvm-pr<wbr>oject?rev=315402&view=rev</a><br>
Log:<br>
[modules] Only take visible using-directives into
account during name lookup.<br>
<br>
Added:<br>
cfe/trunk/test/Modules/using-<wbr>directive.cpp<br>
Modified:<br>
cfe/trunk/lib/Sema/<wbr>SemaLookup.cpp<br>
<br>
Modified: cfe/trunk/lib/Sema/SemaLookup.<wbr>cpp<br>
URL: <a
href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaLookup.cpp?rev=315402&r1=315401&r2=315402&view=diff"
rel="noreferrer" target="_blank"
moz-do-not-send="true">http://llvm.org/viewvc/llvm-pr<wbr>oject/cfe/trunk/lib/Sema/SemaL<wbr>ookup.cpp?rev=315402&r1=315401<wbr>&r2=315402&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/lib/Sema/SemaLookup.<wbr>cpp
(original)<br>
+++ cfe/trunk/lib/Sema/SemaLookup.<wbr>cpp Tue Oct
10 18:19:11 2017<br>
@@ -88,13 +88,15 @@ namespace {<br>
/// A collection of using directives, as used by
C++ unqualified<br>
/// lookup.<br>
class UnqualUsingDirectiveSet {<br>
+ Sema &SemaRef;<br>
+<br>
typedef SmallVector<UnqualUsingEntry, 8>
ListTy;<br>
ListTy list;<br>
llvm::SmallPtrSet<DeclContext*<wbr>, 8>
visited;<br>
public:<br>
- UnqualUsingDirectiveSet() {}<br>
+ UnqualUsingDirectiveSet(Sema &SemaRef) :
SemaRef(SemaRef) {}<br>
void visitScopeChain(Scope *S, Scope
*InnermostFileScope) {<br>
// C++ [namespace.udir]p1:<br>
@@ -113,7 +115,8 @@ namespace {<br>
visit(Ctx, Ctx);<br>
} else if (!Ctx ||
Ctx->isFunctionOrMethod()) {<br>
for (auto *I : S->using_directives())<br>
- visit(I, InnermostFileDC);<br>
+ if (SemaRef.isVisible(I))<br>
+ visit(I, InnermostFileDC);<br>
}<br>
}<br>
}<br>
@@ -152,7 +155,7 @@ namespace {<br>
while (true) {<br>
for (auto UD : DC->using_directives())
{<br>
DeclContext *NS =
UD->getNominatedNamespace();<br>
- if (visited.insert(NS).second) {<br>
+ if (visited.insert(NS).second &&
SemaRef.isVisible(UD)) {<br>
</blockquote>
</div>
</div>
It looks like this change breaks examples such as:<br>
<br>
// RUN: rm -fr %t<br>
// RUN %clang_cc1 -fmodules -fmodules-local-submodule-visi<wbr>bility
-fmodules-cache-path=%t -verify %s<br>
// expected-no-diagnostics<br>
#pragma clang module build M<br>
module M { module TDFNodes {} module TDFInterface {} }<br>
#pragma clang module contents<br>
// TDFNodes<br>
#pragma clang module begin M.TDFNodes<br>
namespace Detail {<br>
namespace TDF {<br>
class TLoopManager {};<br>
}<br>
}<br>
namespace Internal {<br>
namespace TDF {<br>
using namespace Detail::TDF;<br>
}<br>
}<br>
#pragma clang module end<br>
<br>
// TDFInterface<br>
#pragma clang module begin M.TDFInterface<br>
#pragma clang module import M.TDFNodes<br>
namespace Internal {<br>
namespace TDF {<br>
using namespace Detail::TDF;<br>
}<br>
}<br>
#pragma clang module end<br>
<br>
#pragma clang module endbuild<br>
<br>
#pragma clang module import M.TDFNodes<br>
namespace Internal {<br>
namespace TDF {<br>
TLoopManager * use;<br>
}<br>
}<br>
<br>
I suspect that it triggers a preexisting bug in failing
to make M.TDFNodes visible...</blockquote>
<div><br>
</div>
<div>This code in NamedDecl::declarationReplaces is wrong,
we even have a FIXME for the bug:</div>
<div><br>
</div>
<div>
<div> // UsingDirectiveDecl's are not really NamedDecl's,
and all have same name.</div>
<div> // They can be replaced if they nominate the same
namespace.</div>
<div> // FIXME: Is this true even if they have different
module visibility?</div>
<div> if (auto *UD =
dyn_cast<UsingDirectiveDecl>(this))</div>
<div> return
UD->getNominatedNamespace()->getOriginalNamespace()
==</div>
<div>
cast<UsingDirectiveDecl>(OldD)->getNominatedNamespace()</div>
<div> ->getOriginalNamespace();</div>
</div>
<div><br>
</div>
<div>Fixed in r316965 by deleting the above code :)</div>
</div>
</div>
</div>
</blockquote>
You ninjaed me I just understood that the getOriginalNamespace was
wrong and hunting that down :D<br>
<br>
Thanks for the quick fix!<br>
<blockquote type="cite"
cite="mid:CAOfiQqkRvig2c8EZR_9-xbrW7i8u9JjXnFtJtSTLFu=U5nebJg@mail.gmail.com">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote">
<div><br>
</div>
<div>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.</div>
<div> <br>
</div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px
0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">
<div class="gmail-HOEnZb">
<div class="gmail-h5">
<blockquote class="gmail_quote" style="margin:0px 0px
0px 0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">
addUsingDirective(UD, EffectiveDC);<br>
queue.push_back(NS);<br>
}<br>
@@ -1085,7 +1088,7 @@ bool
Sema::CppLookupName(LookupResu<wbr>lt &R<br>
// }<br>
// }<br>
//<br>
- UnqualUsingDirectiveSet UDirs;<br>
+ UnqualUsingDirectiveSet UDirs(*this);<br>
bool VisitedUsingDirectives = false;<br>
bool LeftStartingScope = false;<br>
DeclContext *OutsideOfTemplateParamDC = nullptr;<br>
@@ -1868,22 +1871,19 @@ static bool
LookupQualifiedNameInUsingDi<br>
DeclContext *StartDC) {<br>
assert(StartDC->isFileContext(<wbr>)
&& "start context is not a file context");<br>
- DeclContext::udir_range UsingDirectives =
StartDC->using_directives();<br>
- if (UsingDirectives.begin() ==
UsingDirectives.end()) return false;<br>
+ // We have not yet looked into these namespaces,
much less added<br>
+ // their "using-children" to the queue.<br>
+ SmallVector<NamespaceDecl*, 8> Queue;<br>
// We have at least added all these contexts
to the queue.<br>
llvm::SmallPtrSet<DeclContext*<wbr>, 8>
Visited;<br>
Visited.insert(StartDC);<br>
- // We have not yet looked into these
namespaces, much less added<br>
- // their "using-children" to the queue.<br>
- SmallVector<NamespaceDecl*, 8> Queue;<br>
-<br>
// We have already looked into the initial
namespace; seed the queue<br>
// with its using-children.<br>
- for (auto *I : UsingDirectives) {<br>
+ for (auto *I : StartDC->using_directives()) {<br>
NamespaceDecl *ND =
I->getNominatedNamespace()->ge<wbr>tOriginalNamespace();<br>
- if (Visited.insert(ND).second)<br>
+ if (Visited.insert(ND).second &&
S.isVisible(I))<br>
Queue.push_back(ND);<br>
}<br>
@@ -1931,7 +1931,7 @@ static bool
LookupQualifiedNameInUsingDi<br>
for (auto I : ND->using_directives()) {<br>
NamespaceDecl *Nom =
I->getNominatedNamespace();<br>
- if (Visited.insert(Nom).second)<br>
+ if (Visited.insert(Nom).second &&
S.isVisible(I))<br>
Queue.push_back(Nom);<br>
}<br>
}<br>
@@ -3540,6 +3540,8 @@ static void
LookupVisibleDecls(DeclConte<br>
if (QualifiedNameLookup) {<br>
ShadowContextRAII Shadow(Visited);<br>
for (auto I : Ctx->using_directives()) {<br>
+ if (!Result.getSema().isVisible(I<wbr>))<br>
+ continue;<br>
LookupVisibleDecls(I->getNomin<wbr>atedNamespace(),
Result,<br>
QualifiedNameLookup,
InBaseClass, Consumer, Visited,<br>
IncludeDependentBases);<br>
@@ -3746,7 +3748,7 @@ void
Sema::LookupVisibleDecls(Scope *S,<br>
// Determine the set of using directives
available during<br>
// unqualified name lookup.<br>
Scope *Initial = S;<br>
- UnqualUsingDirectiveSet UDirs;<br>
+ UnqualUsingDirectiveSet UDirs(*this);<br>
if (getLangOpts().CPlusPlus) {<br>
// Find the first namespace or
translation-unit scope.<br>
while (S &&
!isNamespaceOrTranslationUnitS<wbr>cope(S))<br>
<br>
Added: cfe/trunk/test/Modules/using-d<wbr>irective.cpp<br>
URL: <a
href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/using-directive.cpp?rev=315402&view=auto"
rel="noreferrer" target="_blank"
moz-do-not-send="true">http://llvm.org/viewvc/llvm-pr<wbr>oject/cfe/trunk/test/Modules/<wbr>using-directive.cpp?rev=<wbr>315402&view=auto</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/test/Modules/using-d<wbr>irective.cpp
(added)<br>
+++ cfe/trunk/test/Modules/using-d<wbr>irective.cpp
Tue Oct 10 18:19:11 2017<br>
@@ -0,0 +1,62 @@<br>
+// RUN: %clang_cc1 -fmodules
-fmodules-local-submodule-visi<wbr>bility
-fno-modules-error-recovery -fno-spell-checking
-verify %s<br>
+<br>
+#pragma clang module build a<br>
+module a { explicit module b {} explicit module c
{} }<br>
+#pragma clang module contents<br>
+<br>
+#pragma clang module begin a.b<br>
+namespace b { int n; }<br>
+#pragma clang module end<br>
+<br>
+#pragma clang module begin a.c<br>
+#pragma clang module import a.b<br>
+namespace c { using namespace b; }<br>
+#pragma clang module end<br>
+<br>
+#pragma clang module begin a<br>
+#pragma clang module import a.c<br>
+using namespace c;<br>
+#pragma clang module end<br>
+<br>
+#pragma clang module endbuild<br>
+<br>
+#pragma clang module import a.b<br>
+void use1() {<br>
+ (void)n; // expected-error {{use of undeclared
identifier}}<br>
+ (void)::n; // expected-error {{no member named
'n' in the global namespace}}<br>
+ (void)b::n;<br>
+}<br>
+namespace b {<br>
+ void use1_in_b() { (void)n; }<br>
+}<br>
+namespace c {<br>
+ void use1_in_c() { (void)n; } // expected-error
{{use of undeclared identifier}}<br>
+}<br>
+<br>
+#pragma clang module import a.c<br>
+void use2() {<br>
+ (void)n; // expected-error {{use of undeclared
identifier}}<br>
+ (void)::n; // expected-error {{no member named
'n' in the global namespace}}<br>
+ (void)b::n;<br>
+ (void)c::n;<br>
+}<br>
+namespace b {<br>
+ void use2_in_b() { (void)n; }<br>
+}<br>
+namespace c {<br>
+ void use2_in_c() { (void)n; }<br>
+}<br>
+<br>
+#pragma clang module import a<br>
+void use3() {<br>
+ (void)n;<br>
+ (void)::n;<br>
+ (void)b::n;<br>
+ (void)c::n;<br>
+}<br>
+namespace b {<br>
+ void use3_in_b() { (void)n; }<br>
+}<br>
+namespace c {<br>
+ void use3_in_c() { (void)n; }<br>
+}<br>
<br>
<br>
______________________________<wbr>_________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org"
target="_blank" moz-do-not-send="true">cfe-commits@lists.llvm.org</a><br>
<a
href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits"
rel="noreferrer" target="_blank"
moz-do-not-send="true">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/cfe-commits</a><br>
</blockquote>
<br>
<br>
______________________________<wbr>_________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org"
target="_blank" moz-do-not-send="true">cfe-commits@lists.llvm.org</a><br>
<a
href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits"
rel="noreferrer" target="_blank"
moz-do-not-send="true">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/cfe-commits</a><br>
</div>
</div>
</blockquote>
</div>
<br>
</div>
</div>
</blockquote>
<p><br>
</p>
</body>
</html>