Skip to content

Commit

Permalink
[Clang] Fix Unevaluated Lambdas
Browse files Browse the repository at this point in the history
Unlike other types, when lambdas are instanciated,
they are recreated from scratch.
When an unevaluated lambdas appear in the type of a function,
parameter it is instanciated in the wrong declaration context,
as parameters are transformed before the function.

To support lambda in function parameters, we try to
compute whether they are dependant without looking at the
declaration context.

This is a short term stopgap solution to avoid clang
iceing. A better fix might be to inject some kind of
transparent declaration with correctly computed dependency
for function parameters, variable templates, etc.

Fixes llvm#50376
Fixes llvm#51414
Fixes llvm#51416
Fixes llvm#51641
Fixes llvm#54296

Reviewed By: aaron.ballman

Differential Revision: https://reviews.llvm.org/D121532

(cherry picked from commit 3784e8c)
  • Loading branch information
cor3ntin authored and llvmbot committed Apr 2, 2022
1 parent 0547ffe commit ae01c74
Show file tree
Hide file tree
Showing 14 changed files with 161 additions and 41 deletions.
8 changes: 8 additions & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,14 @@ Major New Features
- ``float.h`` now exposes (in hosted mode) extensions made available from the
AIX system header.

- Unevaluated lambdas in dependant contexts no longer result in clang crashing.
This fixes Issues `50376 < https://github.com/llvm/llvm-project/issues/50376>`_,
`54296 < https://github.com/llvm/llvm-project/issues/54296>`_,
`51414 < https://github.com/llvm/llvm-project/issues/51414>`_,
`51416 < https://github.com/llvm/llvm-project/issues/51416>`_,
and `51641 < https://github.com/llvm/llvm-project/issues/51641>`_.


Improvements to Clang's diagnostics
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Expand Down
28 changes: 23 additions & 5 deletions clang/include/clang/AST/DeclCXX.h
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,14 @@ class CXXRecordDecl : public RecordDecl {
SMF_All = 0x3f
};

public:
enum LambdaDependencyKind {
LDK_Unknown = 0,
LDK_AlwaysDependent,
LDK_NeverDependent,
};

private:
struct DefinitionData {
#define FIELD(Name, Width, Merge) \
unsigned Name : Width;
Expand Down Expand Up @@ -374,7 +382,7 @@ class CXXRecordDecl : public RecordDecl {
/// lambda will have been created with the enclosing context as its
/// declaration context, rather than function. This is an unfortunate
/// artifact of having to parse the default arguments before.
unsigned Dependent : 1;
unsigned DependencyKind : 2;

/// Whether this lambda is a generic lambda.
unsigned IsGenericLambda : 1;
Expand Down Expand Up @@ -408,9 +416,9 @@ class CXXRecordDecl : public RecordDecl {
/// The type of the call method.
TypeSourceInfo *MethodTyInfo;

LambdaDefinitionData(CXXRecordDecl *D, TypeSourceInfo *Info, bool Dependent,
LambdaDefinitionData(CXXRecordDecl *D, TypeSourceInfo *Info, unsigned DK,
bool IsGeneric, LambdaCaptureDefault CaptureDefault)
: DefinitionData(D), Dependent(Dependent), IsGenericLambda(IsGeneric),
: DefinitionData(D), DependencyKind(DK), IsGenericLambda(IsGeneric),
CaptureDefault(CaptureDefault), NumCaptures(0),
NumExplicitCaptures(0), HasKnownInternalLinkage(0), ManglingNumber(0),
MethodTyInfo(Info) {
Expand Down Expand Up @@ -547,7 +555,7 @@ class CXXRecordDecl : public RecordDecl {
bool DelayTypeCreation = false);
static CXXRecordDecl *CreateLambda(const ASTContext &C, DeclContext *DC,
TypeSourceInfo *Info, SourceLocation Loc,
bool DependentLambda, bool IsGeneric,
unsigned DependencyKind, bool IsGeneric,
LambdaCaptureDefault CaptureDefault);
static CXXRecordDecl *CreateDeserialized(const ASTContext &C, unsigned ID);

Expand Down Expand Up @@ -1774,7 +1782,17 @@ class CXXRecordDecl : public RecordDecl {
/// function declaration itself is dependent. This flag indicates when we
/// know that the lambda is dependent despite that.
bool isDependentLambda() const {
return isLambda() && getLambdaData().Dependent;
return isLambda() && getLambdaData().DependencyKind == LDK_AlwaysDependent;
}

bool isNeverDependentLambda() const {
return isLambda() && getLambdaData().DependencyKind == LDK_NeverDependent;
}

unsigned getLambdaDependencyKind() const {
if (!isLambda())
return LDK_Unknown;
return getLambdaData().DependencyKind;
}

TypeSourceInfo *getLambdaTypeInfo() const {
Expand Down
2 changes: 1 addition & 1 deletion clang/include/clang/Sema/Sema.h
Original file line number Diff line number Diff line change
Expand Up @@ -6737,7 +6737,7 @@ class Sema final {
/// Create a new lambda closure type.
CXXRecordDecl *createLambdaClosureType(SourceRange IntroducerRange,
TypeSourceInfo *Info,
bool KnownDependent,
unsigned LambdaDependencyKind,
LambdaCaptureDefault CaptureDefault);

/// Start the definition of a lambda expression.
Expand Down
2 changes: 1 addition & 1 deletion clang/lib/AST/ASTImporter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2865,7 +2865,7 @@ ExpectedDecl ASTNodeImporter::VisitRecordDecl(RecordDecl *D) {
return TInfoOrErr.takeError();
if (GetImportedOrCreateSpecialDecl(
D2CXX, CXXRecordDecl::CreateLambda, D, Importer.getToContext(),
DC, *TInfoOrErr, Loc, DCXX->isDependentLambda(),
DC, *TInfoOrErr, Loc, DCXX->getLambdaDependencyKind(),
DCXX->isGenericLambda(), DCXX->getLambdaCaptureDefault()))
return D2CXX;
ExpectedDecl CDeclOrErr = import(DCXX->getLambdaContextDecl());
Expand Down
2 changes: 2 additions & 0 deletions clang/lib/AST/DeclBase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1161,6 +1161,8 @@ bool DeclContext::isDependentContext() const {

if (Record->isDependentLambda())
return true;
if (Record->isNeverDependentLambda())
return false;
}

if (const auto *Function = dyn_cast<FunctionDecl>(this)) {
Expand Down
8 changes: 4 additions & 4 deletions clang/lib/AST/DeclCXX.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -146,16 +146,16 @@ CXXRecordDecl *CXXRecordDecl::Create(const ASTContext &C, TagKind TK,
CXXRecordDecl *
CXXRecordDecl::CreateLambda(const ASTContext &C, DeclContext *DC,
TypeSourceInfo *Info, SourceLocation Loc,
bool Dependent, bool IsGeneric,
unsigned DependencyKind, bool IsGeneric,
LambdaCaptureDefault CaptureDefault) {
auto *R = new (C, DC) CXXRecordDecl(CXXRecord, TTK_Class, C, DC, Loc, Loc,
nullptr, nullptr);
R->setBeingDefined(true);
R->DefinitionData =
new (C) struct LambdaDefinitionData(R, Info, Dependent, IsGeneric,
CaptureDefault);
R->DefinitionData = new (C) struct LambdaDefinitionData(
R, Info, DependencyKind, IsGeneric, CaptureDefault);
R->setMayHaveOutOfDateDef(false);
R->setImplicit(true);

C.getTypeDeclType(R, /*PrevDecl=*/nullptr);
return R;
}
Expand Down
31 changes: 15 additions & 16 deletions clang/lib/Sema/SemaLambda.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -238,21 +238,19 @@ getGenericLambdaTemplateParameterList(LambdaScopeInfo *LSI, Sema &SemaRef) {
return LSI->GLTemplateParameterList;
}

CXXRecordDecl *Sema::createLambdaClosureType(SourceRange IntroducerRange,
TypeSourceInfo *Info,
bool KnownDependent,
LambdaCaptureDefault CaptureDefault) {
CXXRecordDecl *
Sema::createLambdaClosureType(SourceRange IntroducerRange, TypeSourceInfo *Info,
unsigned LambdaDependencyKind,
LambdaCaptureDefault CaptureDefault) {
DeclContext *DC = CurContext;
while (!(DC->isFunctionOrMethod() || DC->isRecord() || DC->isFileContext()))
DC = DC->getParent();
bool IsGenericLambda = getGenericLambdaTemplateParameterList(getCurLambda(),
*this);
// Start constructing the lambda class.
CXXRecordDecl *Class = CXXRecordDecl::CreateLambda(Context, DC, Info,
IntroducerRange.getBegin(),
KnownDependent,
IsGenericLambda,
CaptureDefault);
CXXRecordDecl *Class = CXXRecordDecl::CreateLambda(
Context, DC, Info, IntroducerRange.getBegin(), LambdaDependencyKind,
IsGenericLambda, CaptureDefault);
DC->addDecl(Class);

return Class;
Expand Down Expand Up @@ -898,17 +896,18 @@ void Sema::ActOnStartOfLambdaDefinition(LambdaIntroducer &Intro,

// Determine if we're within a context where we know that the lambda will
// be dependent, because there are template parameters in scope.
bool KnownDependent;
CXXRecordDecl::LambdaDependencyKind LambdaDependencyKind =
CXXRecordDecl::LDK_Unknown;
if (LSI->NumExplicitTemplateParams > 0) {
auto *TemplateParamScope = CurScope->getTemplateParamParent();
assert(TemplateParamScope &&
"Lambda with explicit template param list should establish a "
"template param scope");
assert(TemplateParamScope->getParent());
KnownDependent = TemplateParamScope->getParent()
->getTemplateParamParent() != nullptr;
} else {
KnownDependent = CurScope->getTemplateParamParent() != nullptr;
if (TemplateParamScope->getParent()->getTemplateParamParent() != nullptr)
LambdaDependencyKind = CXXRecordDecl::LDK_AlwaysDependent;
} else if (CurScope->getTemplateParamParent() != nullptr) {
LambdaDependencyKind = CXXRecordDecl::LDK_AlwaysDependent;
}

// Determine the signature of the call operator.
Expand Down Expand Up @@ -977,8 +976,8 @@ void Sema::ActOnStartOfLambdaDefinition(LambdaIntroducer &Intro,
UPPC_DeclarationType);
}

CXXRecordDecl *Class = createLambdaClosureType(Intro.Range, MethodTyInfo,
KnownDependent, Intro.Default);
CXXRecordDecl *Class = createLambdaClosureType(
Intro.Range, MethodTyInfo, LambdaDependencyKind, Intro.Default);
CXXMethodDecl *Method =
startLambdaDefinition(Class, Intro.Range, MethodTyInfo, EndLoc, Params,
ParamInfo.getDeclSpec().getConstexprSpecifier(),
Expand Down
2 changes: 1 addition & 1 deletion clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1831,7 +1831,7 @@ Decl *TemplateDeclInstantiator::VisitCXXRecordDecl(CXXRecordDecl *D) {
if (D->isLambda())
Record = CXXRecordDecl::CreateLambda(
SemaRef.Context, Owner, D->getLambdaTypeInfo(), D->getLocation(),
D->isDependentLambda(), D->isGenericLambda(),
D->getLambdaDependencyKind(), D->isGenericLambda(),
D->getLambdaCaptureDefault());
else
Record = CXXRecordDecl::Create(SemaRef.Context, D->getTagKind(), Owner,
Expand Down
24 changes: 17 additions & 7 deletions clang/lib/Sema/TreeTransform.h
Original file line number Diff line number Diff line change
Expand Up @@ -12922,14 +12922,24 @@ TreeTransform<Derived>::TransformLambdaExpr(LambdaExpr *E) {
NewTrailingRequiresClause = getDerived().TransformExpr(TRC);

// Create the local class that will describe the lambda.
// FIXME: KnownDependent below is wrong when substituting inside a templated
// context that isn't a DeclContext (such as a variable template).

// FIXME: DependencyKind below is wrong when substituting inside a templated
// context that isn't a DeclContext (such as a variable template), or when
// substituting an unevaluated lambda inside of a function's parameter's type
// - as parameter types are not instantiated from within a function's DC. We
// use isUnevaluatedContext() to distinguish the function parameter case.
CXXRecordDecl::LambdaDependencyKind DependencyKind =
CXXRecordDecl::LDK_Unknown;
if (getSema().isUnevaluatedContext() &&
(getSema().CurContext->isFileContext() ||
!getSema().CurContext->getParent()->isDependentContext()))
DependencyKind = CXXRecordDecl::LDK_NeverDependent;

CXXRecordDecl *OldClass = E->getLambdaClass();
CXXRecordDecl *Class
= getSema().createLambdaClosureType(E->getIntroducerRange(),
NewCallOpTSI,
/*KnownDependent=*/false,
E->getCaptureDefault());
CXXRecordDecl *Class =
getSema().createLambdaClosureType(E->getIntroducerRange(), NewCallOpTSI,
DependencyKind, E->getCaptureDefault());

getDerived().transformedLocalDecl(OldClass, {Class});

Optional<std::tuple<bool, unsigned, unsigned, Decl *>> Mangling;
Expand Down
6 changes: 3 additions & 3 deletions clang/lib/Serialization/ASTReaderDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1801,7 +1801,7 @@ void ASTDeclReader::ReadCXXDefinitionData(
using Capture = LambdaCapture;

auto &Lambda = static_cast<CXXRecordDecl::LambdaDefinitionData &>(Data);
Lambda.Dependent = Record.readInt();
Lambda.DependencyKind = Record.readInt();
Lambda.IsGenericLambda = Record.readInt();
Lambda.CaptureDefault = Record.readInt();
Lambda.NumCaptures = Record.readInt();
Expand Down Expand Up @@ -1917,8 +1917,8 @@ void ASTDeclReader::ReadCXXRecordDefinition(CXXRecordDecl *D, bool Update) {
// allocate the appropriate DefinitionData structure.
bool IsLambda = Record.readInt();
if (IsLambda)
DD = new (C) CXXRecordDecl::LambdaDefinitionData(D, nullptr, false, false,
LCD_None);
DD = new (C) CXXRecordDecl::LambdaDefinitionData(
D, nullptr, CXXRecordDecl::LDK_Unknown, false, LCD_None);
else
DD = new (C) struct CXXRecordDecl::DefinitionData(D);

Expand Down
2 changes: 1 addition & 1 deletion clang/lib/Serialization/ASTWriter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5726,7 +5726,7 @@ void ASTRecordWriter::AddCXXDefinitionData(const CXXRecordDecl *D) {
// Add lambda-specific data.
if (Data.IsLambda) {
auto &Lambda = D->getLambdaData();
Record->push_back(Lambda.Dependent);
Record->push_back(Lambda.DependencyKind);
Record->push_back(Lambda.IsGenericLambda);
Record->push_back(Lambda.CaptureDefault);
Record->push_back(Lambda.NumCaptures);
Expand Down
79 changes: 78 additions & 1 deletion clang/test/SemaCXX/lambda-unevaluated.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,27 @@ auto g(T) -> decltype([]() { T::invalid; } ());
auto e = g(0); // expected-error{{no matching function for call}}
// expected-note@-2 {{substitution failure}}

template <typename T>
auto foo(decltype([] {
return [] { return T(); }();
})) {}

void test() {
foo<int>({});
}

template <typename T>
struct C {
template <typename U>
auto foo(decltype([] {
return [] { return T(); }();
})) {}
};

void test2() {
C<int>{}.foo<long>({});
}

namespace PR52073 {
// OK, these are distinct functions not redefinitions.
template<typename> void f(decltype([]{})) {} // expected-note {{candidate}}
Expand All @@ -40,6 +61,62 @@ void use_f() { f<int>({}); } // expected-error {{ambiguous}}
template<int N> void g(const char (*)[([]{ return N; })()]) {} // expected-note {{candidate}}
template<int N> void g(const char (*)[([]{ return N; })()]) {} // expected-note {{candidate}}
// FIXME: We instantiate the lambdas into the context of the function template,
// so we think they're dependent and can't evaluate a call to them.
// so we think they're dependent and can't evaluate a call to them.
void use_g() { g<6>(&"hello"); } // expected-error {{no matching function}}
}

namespace GH51416 {

template <class T>
struct A {
void spam(decltype([] {}));
};

template <class T>
void A<T>::spam(decltype([] {})) // expected-error{{out-of-line definition of 'spam' does not match}}
{}

struct B {
template <class T>
void spam(decltype([] {}));
};

template <class T>
void B::spam(decltype([] {})) {} // expected-error{{out-of-line definition of 'spam' does not match}}

} // namespace GH51416

namespace GH50376 {

template <typename T, typename Fn>
struct foo_t { // expected-note 2{{candidate constructor}}
foo_t(T ptr) {} // expected-note{{candidate constructor}}
};

template <typename T>
using alias = foo_t<T, decltype([](int) { return 0; })>;

template <typename T>
auto fun(T const &t) -> alias<T> {
return alias<T>{t}; // expected-error{{no viable conversion from returned value of type 'alias<...>'}}
}

void f() {
int i;
auto const error = fun(i); // expected-note{{in instantiation}}
}

} // namespace GH50376

namespace GH51414 {
template <class T> void spam(decltype([] {}) (*s)[sizeof(T)] = nullptr) {}
void foo() {
spam<int>();
}
} // namespace GH51414

namespace GH51641 {
template <class T>
void foo(decltype(+[](T) {}) lambda, T param);
static_assert(!__is_same(decltype(foo<int>), void));
} // namespace GH51641
2 changes: 2 additions & 0 deletions clang/unittests/AST/ASTImporterTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5824,6 +5824,7 @@ TEST_P(ASTImporterOptionSpecificTestBase, LambdaInFunctionBody) {
std::distance(FromL->decls().begin(), FromL->decls().end());
EXPECT_NE(ToLSize, 0u);
EXPECT_EQ(ToLSize, FromLSize);
EXPECT_FALSE(FromL->isDependentLambda());
}

TEST_P(ASTImporterOptionSpecificTestBase, LambdaInFunctionParam) {
Expand All @@ -5843,6 +5844,7 @@ TEST_P(ASTImporterOptionSpecificTestBase, LambdaInFunctionParam) {
std::distance(FromL->decls().begin(), FromL->decls().end());
EXPECT_NE(ToLSize, 0u);
EXPECT_EQ(ToLSize, FromLSize);
EXPECT_TRUE(FromL->isDependentLambda());
}

TEST_P(ASTImporterOptionSpecificTestBase, LambdaInGlobalScope) {
Expand Down
6 changes: 5 additions & 1 deletion clang/www/cxx_status.html
Original file line number Diff line number Diff line change
Expand Up @@ -1021,7 +1021,11 @@ <h2 id="cxx20">C++20 implementation status</h2>
<tr>
<td>Lambdas in unevaluated contexts</td>
<td><a href="https://wg21.link/p0315r4">P0315R4</a></td>
<td class="partial" align="center">Partial</td>
<td class="partial" align="center">
<details><summary>Partial</summary>
temp.deduct/9 is not implemented yet.
</details>
</td>
</tr>
<!-- Jacksonville papers -->
<tr>
Expand Down

0 comments on commit ae01c74

Please sign in to comment.