Re: r313955 - Give external linkage and mangling to lambdas inside inline variables and variable templates.
On 23 September 2017 at 01:18, NAKAMURA Takumi via cfe-commits <
cfe-commits@lists.llvm.org> wrote:
> It seems libcxx (with -fmodules) dislikes this change. Any idea?
> http://bb.pgr.jp/builders/bootstrap-clang-libcxx-lld-i686-linux/builds/238
>
The code does this:
template struct X {
static const unsigned M = (N == 32) ? ~0U : (1U << N) - 1;
};
X<32> x32;
... which seems pretty reasonable -- instantiating with N = 32 does not
reach the "1U << 32" portion. However, Clang's warning suppression for
unreachable code doesn't work for the initializer of a static data member,
and this change fixed a bug where we'd incorrectly treat such an
initializer as a constant-evaluated context, which happened to be hiding
the warning.
(The non-templated case:
struct X {
static const unsigned M = (32 == 32) ? ~0U : (1U << 32) - 1;
};
... warned even before this change, but the function-scope version:
unsigned f() {
return (32 == 32) ? ~0U : (1U << 32) - 1;
}
... does not, because we prune unreachable warnings within functions.)
Long-term, the right thing to do is to build a CFG for variable
initializers like we do for functions. But in the short term we can at
least suppress "runtime behavior" diagnostics in variable initializers that
will always be evaluated as constants (constexpr variables and in-class
initializers of static data members). I've implemented that in r314067.
On Fri, Sep 22, 2017 at 1:26 PM Richard Smith via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Author: rsmith
>> Date: Thu Sep 21 21:25:05 2017
>> New Revision: 313955
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=313955&view=rev
>> Log:
>> Give external linkage and mangling to lambdas inside inline variables and
>> variable templates.
>>
>> This implements the proposed approach in https://github.com/itanium-
>> cxx-abi/cxx-abi/issues/33
>>
>> This reinstates r313827, reverted in r313856, with a fix for the
>> 'out-of-bounds
>> enumeration value' ubsan error in that change.
>>
>> Modified:
>> cfe/trunk/lib/AST/Decl.cpp
>> cfe/trunk/lib/AST/ItaniumMangle.cpp
>> cfe/trunk/lib/AST/Linkage.h
>> cfe/trunk/lib/Parse/ParseDecl.cpp
>> cfe/trunk/lib/Sema/SemaDeclCXX.cpp
>> cfe/trunk/lib/Sema/SemaLambda.cpp
>> cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
>> cfe/trunk/test/CodeGenCXX/mangle-lambdas.cpp
>> cfe/trunk/test/SemaCXX/vartemplate-lambda.cpp
>> cfe/trunk/test/SemaTemplate/instantiate-static-var.cpp
>>
>> Modified: cfe/trunk/lib/AST/Decl.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/
>> Decl.cpp?rev=313955&r1=313954&r2=313955&view=diff
>>
>> ==
>> --- cfe/trunk/lib/AST/Decl.cpp (original)
>> +++ cfe/trunk/lib/AST/Decl.cpp Thu Sep 21 21:25:05 2017
>> @@ -554,7 +554,8 @@ static LinkageInfo getExternalLinkageFor
>>
>> LinkageInfo
>> LinkageComputer::getLVForNamespaceScopeDecl(const NamedDecl *D,
>> -LVComputationKind
>> computation) {
>> +LVComputationKind
>> computation,
>> +bool IgnoreVarTypeLinkage) {
>>assert(D->getDeclContext()->getRedeclContext()->isFileContext() &&
>> "Not a name having namespace scope");
>>ASTContext &Context = D->getASTContext();
>> @@ -611,7 +612,7 @@ LinkageComputer::getLVForNamespaceScopeD
>> // - a data member of an anonymous union.
>> const VarDecl *VD = IFD->getVarDecl();
>> assert(VD && "Expected a VarDecl in this IndirectFieldDecl!");
>> -return getLVForNamespaceScopeDecl(VD, computation);
>> +return getLVForNamespaceScopeDecl(VD, computation,
>> IgnoreVarTypeLinkage);
>>}
>>assert(!isa(D) && "Didn't expect a FieldDecl!");
>>
>> @@ -700,7 +701,8 @@ LinkageComputer::getLVForNamespaceScopeD
>> //
>> // Note that we don't want to make the variable non-external
>> // because of this, but unique-external linkage suits us.
>> -if (Context.getLangOpts().CPlusPlus &&
>> !isFirstInExternCContext(Var)) {
>> +if (Context.getLangOpts().CPlusPlus &&
>> !isFirstInExternCContext(Var) &&
>> +!IgnoreVarTypeLinkage) {
>>LinkageInfo TypeLV = getLVForType(*Var->getType(), computation);
>>if (!isExternallyVisible(TypeLV.getLinkage()))
>> return LinkageInfo::uniqueExternal();
>> @@ -740,15 +742,9 @@ LinkageComputer::getLVForNamespaceScopeD
>> // unique-external linkage, it's not legally usable from outside
>> // this translation unit. However, we should use the C linkage
>> // rules instead for extern "C" declarations.
>> -if (Context.getLangOpts().CPlusPlus &&
>> -!Function->isInExternCContext()) {
>> - // Only look at the type-as-written. If this function has an
>> auto-deduced
>> - // return type, we can't compute the linkage of that type because
>> it could
>> - // require looking at the lin
Re: r313955 - Give external linkage and mangling to lambdas inside inline variables and variable templates.
It seems libcxx (with -fmodules) dislikes this change. Any idea?
http://bb.pgr.jp/builders/bootstrap-clang-libcxx-lld-i686-linux/builds/238
On Fri, Sep 22, 2017 at 1:26 PM Richard Smith via cfe-commits <
cfe-commits@lists.llvm.org> wrote:
> Author: rsmith
> Date: Thu Sep 21 21:25:05 2017
> New Revision: 313955
>
> URL: http://llvm.org/viewvc/llvm-project?rev=313955&view=rev
> Log:
> Give external linkage and mangling to lambdas inside inline variables and
> variable templates.
>
> This implements the proposed approach in
> https://github.com/itanium-cxx-abi/cxx-abi/issues/33
>
> This reinstates r313827, reverted in r313856, with a fix for the
> 'out-of-bounds
> enumeration value' ubsan error in that change.
>
> Modified:
> cfe/trunk/lib/AST/Decl.cpp
> cfe/trunk/lib/AST/ItaniumMangle.cpp
> cfe/trunk/lib/AST/Linkage.h
> cfe/trunk/lib/Parse/ParseDecl.cpp
> cfe/trunk/lib/Sema/SemaDeclCXX.cpp
> cfe/trunk/lib/Sema/SemaLambda.cpp
> cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
> cfe/trunk/test/CodeGenCXX/mangle-lambdas.cpp
> cfe/trunk/test/SemaCXX/vartemplate-lambda.cpp
> cfe/trunk/test/SemaTemplate/instantiate-static-var.cpp
>
> Modified: cfe/trunk/lib/AST/Decl.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Decl.cpp?rev=313955&r1=313954&r2=313955&view=diff
>
> ==
> --- cfe/trunk/lib/AST/Decl.cpp (original)
> +++ cfe/trunk/lib/AST/Decl.cpp Thu Sep 21 21:25:05 2017
> @@ -554,7 +554,8 @@ static LinkageInfo getExternalLinkageFor
>
> LinkageInfo
> LinkageComputer::getLVForNamespaceScopeDecl(const NamedDecl *D,
> -LVComputationKind
> computation) {
> +LVComputationKind computation,
> +bool IgnoreVarTypeLinkage) {
>assert(D->getDeclContext()->getRedeclContext()->isFileContext() &&
> "Not a name having namespace scope");
>ASTContext &Context = D->getASTContext();
> @@ -611,7 +612,7 @@ LinkageComputer::getLVForNamespaceScopeD
> // - a data member of an anonymous union.
> const VarDecl *VD = IFD->getVarDecl();
> assert(VD && "Expected a VarDecl in this IndirectFieldDecl!");
> -return getLVForNamespaceScopeDecl(VD, computation);
> +return getLVForNamespaceScopeDecl(VD, computation,
> IgnoreVarTypeLinkage);
>}
>assert(!isa(D) && "Didn't expect a FieldDecl!");
>
> @@ -700,7 +701,8 @@ LinkageComputer::getLVForNamespaceScopeD
> //
> // Note that we don't want to make the variable non-external
> // because of this, but unique-external linkage suits us.
> -if (Context.getLangOpts().CPlusPlus && !isFirstInExternCContext(Var))
> {
> +if (Context.getLangOpts().CPlusPlus && !isFirstInExternCContext(Var)
> &&
> +!IgnoreVarTypeLinkage) {
>LinkageInfo TypeLV = getLVForType(*Var->getType(), computation);
>if (!isExternallyVisible(TypeLV.getLinkage()))
> return LinkageInfo::uniqueExternal();
> @@ -740,15 +742,9 @@ LinkageComputer::getLVForNamespaceScopeD
> // unique-external linkage, it's not legally usable from outside
> // this translation unit. However, we should use the C linkage
> // rules instead for extern "C" declarations.
> -if (Context.getLangOpts().CPlusPlus &&
> -!Function->isInExternCContext()) {
> - // Only look at the type-as-written. If this function has an
> auto-deduced
> - // return type, we can't compute the linkage of that type because
> it could
> - // require looking at the linkage of this function, and we don't
> need this
> - // for correctness because the type is not part of the function's
> - // signature.
> - // FIXME: This is a hack. We should be able to solve this
> circularity and
> - // the one in getLVForClassMember for Functions some other way.
> +if (Context.getLangOpts().CPlusPlus &&
> !Function->isInExternCContext()) {
> + // Only look at the type-as-written. Otherwise, deducing the return
> type
> + // of a function could change its linkage.
>QualType TypeAsWritten = Function->getType();
>if (TypeSourceInfo *TSI = Function->getTypeSourceInfo())
> TypeAsWritten = TSI->getType();
> @@ -831,7 +827,8 @@ LinkageComputer::getLVForNamespaceScopeD
>
> LinkageInfo
> LinkageComputer::getLVForClassMember(const NamedDecl *D,
> - LVComputationKind computation) {
> + LVComputationKind computation,
> + bool IgnoreVarTypeLinkage) {
>// Only certain class members have linkage. Note that fields don't
>// really have linkage, but it's convenient to say they do for the
>// purposes of calculating linkage of pointer-to-data-member
> @@ -889,22 +886,14 @@ LinkageComputer::getLVForClassMember(con
>const NamedDecl *explici
r313955 - Give external linkage and mangling to lambdas inside inline variables and variable templates.
Author: rsmith
Date: Thu Sep 21 21:25:05 2017
New Revision: 313955
URL: http://llvm.org/viewvc/llvm-project?rev=313955&view=rev
Log:
Give external linkage and mangling to lambdas inside inline variables and
variable templates.
This implements the proposed approach in
https://github.com/itanium-cxx-abi/cxx-abi/issues/33
This reinstates r313827, reverted in r313856, with a fix for the 'out-of-bounds
enumeration value' ubsan error in that change.
Modified:
cfe/trunk/lib/AST/Decl.cpp
cfe/trunk/lib/AST/ItaniumMangle.cpp
cfe/trunk/lib/AST/Linkage.h
cfe/trunk/lib/Parse/ParseDecl.cpp
cfe/trunk/lib/Sema/SemaDeclCXX.cpp
cfe/trunk/lib/Sema/SemaLambda.cpp
cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
cfe/trunk/test/CodeGenCXX/mangle-lambdas.cpp
cfe/trunk/test/SemaCXX/vartemplate-lambda.cpp
cfe/trunk/test/SemaTemplate/instantiate-static-var.cpp
Modified: cfe/trunk/lib/AST/Decl.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Decl.cpp?rev=313955&r1=313954&r2=313955&view=diff
==
--- cfe/trunk/lib/AST/Decl.cpp (original)
+++ cfe/trunk/lib/AST/Decl.cpp Thu Sep 21 21:25:05 2017
@@ -554,7 +554,8 @@ static LinkageInfo getExternalLinkageFor
LinkageInfo
LinkageComputer::getLVForNamespaceScopeDecl(const NamedDecl *D,
-LVComputationKind computation) {
+LVComputationKind computation,
+bool IgnoreVarTypeLinkage) {
assert(D->getDeclContext()->getRedeclContext()->isFileContext() &&
"Not a name having namespace scope");
ASTContext &Context = D->getASTContext();
@@ -611,7 +612,7 @@ LinkageComputer::getLVForNamespaceScopeD
// - a data member of an anonymous union.
const VarDecl *VD = IFD->getVarDecl();
assert(VD && "Expected a VarDecl in this IndirectFieldDecl!");
-return getLVForNamespaceScopeDecl(VD, computation);
+return getLVForNamespaceScopeDecl(VD, computation, IgnoreVarTypeLinkage);
}
assert(!isa(D) && "Didn't expect a FieldDecl!");
@@ -700,7 +701,8 @@ LinkageComputer::getLVForNamespaceScopeD
//
// Note that we don't want to make the variable non-external
// because of this, but unique-external linkage suits us.
-if (Context.getLangOpts().CPlusPlus && !isFirstInExternCContext(Var)) {
+if (Context.getLangOpts().CPlusPlus && !isFirstInExternCContext(Var) &&
+!IgnoreVarTypeLinkage) {
LinkageInfo TypeLV = getLVForType(*Var->getType(), computation);
if (!isExternallyVisible(TypeLV.getLinkage()))
return LinkageInfo::uniqueExternal();
@@ -740,15 +742,9 @@ LinkageComputer::getLVForNamespaceScopeD
// unique-external linkage, it's not legally usable from outside
// this translation unit. However, we should use the C linkage
// rules instead for extern "C" declarations.
-if (Context.getLangOpts().CPlusPlus &&
-!Function->isInExternCContext()) {
- // Only look at the type-as-written. If this function has an auto-deduced
- // return type, we can't compute the linkage of that type because it
could
- // require looking at the linkage of this function, and we don't need
this
- // for correctness because the type is not part of the function's
- // signature.
- // FIXME: This is a hack. We should be able to solve this circularity
and
- // the one in getLVForClassMember for Functions some other way.
+if (Context.getLangOpts().CPlusPlus && !Function->isInExternCContext()) {
+ // Only look at the type-as-written. Otherwise, deducing the return type
+ // of a function could change its linkage.
QualType TypeAsWritten = Function->getType();
if (TypeSourceInfo *TSI = Function->getTypeSourceInfo())
TypeAsWritten = TSI->getType();
@@ -831,7 +827,8 @@ LinkageComputer::getLVForNamespaceScopeD
LinkageInfo
LinkageComputer::getLVForClassMember(const NamedDecl *D,
- LVComputationKind computation) {
+ LVComputationKind computation,
+ bool IgnoreVarTypeLinkage) {
// Only certain class members have linkage. Note that fields don't
// really have linkage, but it's convenient to say they do for the
// purposes of calculating linkage of pointer-to-data-member
@@ -889,22 +886,14 @@ LinkageComputer::getLVForClassMember(con
const NamedDecl *explicitSpecSuppressor = nullptr;
if (const auto *MD = dyn_cast(D)) {
-// If the type of the function uses a type that has non-externally-visible
-// linkage, it's not legally usable from outside this translation unit.
-// But only look at the type-as-written. If this function has an
-// auto-deduced return type, we can't compute the linkage of that type
-// because it could require looking at the linkage of thi
