r315402 - [modules] Only take visible using-directives into account during name lookup.

Vassil Vassilev via cfe-commits cfe-commits at lists.llvm.org
Mon Oct 30 15:12:58 PDT 2017


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
> 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
> ==============================================================================
> --- 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...
>               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
> ==============================================================================
> --- 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 at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits




More information about the cfe-commits mailing list