r316965 - [modules] Retain multiple using-directives in the same scope even if they name the same namespace.

[Richard Smith via cfe-commits]

I should add: the test is courtesy of Vassil Vassilev. Thanks!

On 30 October 2017 at 15:38, Richard Smith via cfe-commits <
cfe-commits at lists.llvm.org> wrote:

> Author: rsmith
> Date: Mon Oct 30 15:38:20 2017
> New Revision: 316965
>
> URL: http://llvm.org/viewvc/llvm-project?rev=316965&view=rev
> Log:
> [modules] Retain multiple using-directives in the same scope even if they
> name the same namespace.
>
> They might have different visibility, and thus discarding all but one of
> them
> can result in rejecting valid code. Also fix name lookup to cope with
> multiple
> using-directives being found that denote the same namespace, where some
> are not
> visible -- don't cache an "already visited" state for a using-directive
> that we
> didn't visit because it was hidden.
>
> Added:
>     cfe/trunk/test/Modules/using-directive-redecl.cpp
> Modified:
>     cfe/trunk/lib/AST/Decl.cpp
>     cfe/trunk/lib/Sema/SemaLookup.cpp
>
> Modified: cfe/trunk/lib/AST/Decl.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/
> Decl.cpp?rev=316965&r1=316964&r2=316965&view=diff
> ============================================================
> ==================
> --- cfe/trunk/lib/AST/Decl.cpp (original)
> +++ cfe/trunk/lib/AST/Decl.cpp Mon Oct 30 15:38:20 2017
> @@ -1597,14 +1597,6 @@ bool NamedDecl::declarationReplaces(Name
>                          cast<UnresolvedUsingValueDecl>
> (OldD)->getQualifier());
>    }
>
> -  // 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();
> -
>    if (isRedeclarable(getKind())) {
>      if (getCanonicalDecl() != OldD->getCanonicalDecl())
>        return false;
>
> Modified: cfe/trunk/lib/Sema/SemaLookup.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/
> SemaLookup.cpp?rev=316965&r1=316964&r2=316965&view=diff
> ============================================================
> ==================
> --- cfe/trunk/lib/Sema/SemaLookup.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaLookup.cpp Mon Oct 30 15:38:20 2017
> @@ -155,7 +155,7 @@ namespace {
>        while (true) {
>          for (auto UD : DC->using_directives()) {
>            DeclContext *NS = UD->getNominatedNamespace();
> -          if (visited.insert(NS).second && SemaRef.isVisible(UD)) {
> +          if (SemaRef.isVisible(UD) && visited.insert(NS).second) {
>              addUsingDirective(UD, EffectiveDC);
>              queue.push_back(NS);
>            }
> @@ -1883,7 +1883,7 @@ static bool LookupQualifiedNameInUsingDi
>    // with its using-children.
>    for (auto *I : StartDC->using_directives()) {
>      NamespaceDecl *ND = I->getNominatedNamespace()->
> getOriginalNamespace();
> -    if (Visited.insert(ND).second && S.isVisible(I))
> +    if (S.isVisible(I) && Visited.insert(ND).second)
>        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 && S.isVisible(I))
> +      if (S.isVisible(I) && Visited.insert(Nom).second)
>          Queue.push_back(Nom);
>      }
>    }
>
> Added: cfe/trunk/test/Modules/using-directive-redecl.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/
> Modules/using-directive-redecl.cpp?rev=316965&view=auto
> ============================================================
> ==================
> --- cfe/trunk/test/Modules/using-directive-redecl.cpp (added)
> +++ cfe/trunk/test/Modules/using-directive-redecl.cpp Mon Oct 30 15:38:20
> 2017
> @@ -0,0 +1,37 @@
> +// RUN: %clang_cc1 -fmodules -fmodules-local-submodule-visibility
> -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;
> +  }
> +}
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20171030/0d3838ac/attachment.html>