r313957 - Closure types have no name (and can't have a typedef name for linkage

Peter Collingbourne via cfe-commits cfe-commits at lists.llvm.org
Mon Oct 2 17:10:39 PDT 2017


Looks like this caused PR34811, which caused a link error on a Chromium bot:
https://build.chromium.org/p/chromium.fyi/builders/CFI%20Linux%20ToT/builds/7081

The link error might be caused by an unrelated LTO bug, but this bug does
seem real. Please take a look.

Peter

On Thu, Sep 21, 2017 at 9:33 PM, Richard Smith via cfe-commits <
cfe-commits at lists.llvm.org> wrote:

> Author: rsmith
> Date: Thu Sep 21 21:33:20 2017
> New Revision: 313957
>
> URL: http://llvm.org/viewvc/llvm-project?rev=313957&view=rev
> Log:
> Closure types have no name (and can't have a typedef name for linkage
> purposes), so they never formally have linkage.
>
> Modified:
>     cfe/trunk/lib/AST/Decl.cpp
>     cfe/trunk/test/CXX/basic/basic.link/p8.cpp
>
> Modified: cfe/trunk/lib/AST/Decl.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/
> Decl.cpp?rev=313957&r1=313956&r2=313957&view=diff
> ============================================================
> ==================
> --- cfe/trunk/lib/AST/Decl.cpp (original)
> +++ cfe/trunk/lib/AST/Decl.cpp Thu Sep 21 21:33:20 2017
> @@ -1104,24 +1104,25 @@ LinkageInfo LinkageComputer::getLVForClo
>    else
>      Owner = cast<NamedDecl>(ContextDecl);
>
> -  // FIXME: If there is no owner, the closure should have no linkage.
>    if (!Owner)
> -    return LinkageInfo::external();
> +    return LinkageInfo::none();
>
>    // If the owner has a deduced type, we need to skip querying the
> linkage and
>    // visibility of that type, because it might involve this closure
> type.  The
>    // only effect of this is that we might give a lambda VisibleNoLinkage
> rather
>    // than NoLinkage when we don't strictly need to, which is benign.
>    auto *VD = dyn_cast<VarDecl>(Owner);
> -  LinkageInfo OwnerLinkage =
> +  LinkageInfo OwnerLV =
>        VD && VD->getType()->getContainedDeducedType()
>            ? computeLVForDecl(Owner, computation,
> /*IgnoreVarTypeLinkage*/true)
>            : getLVForDecl(Owner, computation);
>
> -  // FIXME: This is wrong. A lambda never formally has linkage; if this
> -  // calculation determines a lambda has external linkage, it should be
> -  // downgraded to VisibleNoLinkage.
> -  return OwnerLinkage;
> +  // A lambda never formally has linkage. But if the owner is externally
> +  // visible, then the lambda is too. We apply the same rules to blocks.
> +  if (!isExternallyVisible(OwnerLV.getLinkage()))
> +    return LinkageInfo::none();
> +  return LinkageInfo(VisibleNoLinkage, OwnerLV.getVisibility(),
> +                     OwnerLV.isVisibilityExplicit());
>  }
>
>  LinkageInfo LinkageComputer::getLVForLocalDecl(const NamedDecl *D,
>
> Modified: cfe/trunk/test/CXX/basic/basic.link/p8.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/
> basic/basic.link/p8.cpp?rev=313957&r1=313956&r2=313957&view=diff
> ============================================================
> ==================
> --- cfe/trunk/test/CXX/basic/basic.link/p8.cpp (original)
> +++ cfe/trunk/test/CXX/basic/basic.link/p8.cpp Thu Sep 21 21:33:20 2017
> @@ -14,7 +14,7 @@ typedef decltype(f()) NoLinkage3;
>  inline auto g() { return [] {}; }
>  typedef decltype(g()) VisibleNoLinkage1;
>  inline auto y = [] {};
> -typedef decltype(x) VisibleNoLinkage2;
> +typedef decltype(y) VisibleNoLinkage2;
>  inline auto h() { struct {} x; return x; }
>  typedef decltype(h()) VisibleNoLinkage3;
>
> @@ -42,19 +42,12 @@ void use_no_linkage() {
>    no_linkage3(); // expected-note {{used here}}
>  }
>
> -// FIXME: This should emit an extension warning. It does not because we
> -// incorrectly give the lambda external linkage.
> -extern VisibleNoLinkage1 visible_no_linkage1();
> -
> -// FIXME: We should accept this as an extension. We don't because we
> -// incorrectly give the lambda no linkage instead of "VisibleNoLinkage".
> -extern VisibleNoLinkage2 visible_no_linkage2(); // expected-error {{used
> but not defined}}
> -
> -// This case is correctly accepted as an extension.
> +extern VisibleNoLinkage1 visible_no_linkage1(); // expected-warning {{ISO
> C++ requires a definition}}
> +extern VisibleNoLinkage2 visible_no_linkage2(); // expected-warning {{ISO
> C++ requires a definition}}
>  extern VisibleNoLinkage3 visible_no_linkage3(); // expected-warning {{ISO
> C++ requires a definition}}
>
>  void use_visible_no_linkage() {
> -  visible_no_linkage1();
> +  visible_no_linkage1(); // expected-note {{used here}}
>    visible_no_linkage2(); // expected-note {{used here}}
>    visible_no_linkage3(); // expected-note {{used here}}
>  }
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>



-- 
-- 
Peter
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20171002/7bce24d7/attachment.html>


More information about the cfe-commits mailing list