diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 30cedbe774be9..5708e691d0849 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -513,6 +513,9 @@ Bug Fixes to C++ Support - Fix crash when inheriting from a cv-qualified type. Fixes: (`#35603 `_) - Fix a crash when the using enum declaration uses an anonymous enumeration. Fixes (#GH86790). +- Clang now correctly tracks type dependence of by-value captures in lambdas with an explicit + object parameter. + Fixes (#GH70604), (#GH79754), (#GH84163), (#GH84425), (#GH86054), (#GH86398), and (#GH86399). Bug Fixes to AST Handling ^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/clang/include/clang/AST/ExprCXX.h b/clang/include/clang/AST/ExprCXX.h index 6003b866c9f56..7eb99a75e1fc9 100644 --- a/clang/include/clang/AST/ExprCXX.h +++ b/clang/include/clang/AST/ExprCXX.h @@ -1149,6 +1149,7 @@ class CXXThisExpr : public Expr { CXXThisExpr(SourceLocation L, QualType Ty, bool IsImplicit, ExprValueKind VK) : Expr(CXXThisExprClass, Ty, VK, OK_Ordinary) { CXXThisExprBits.IsImplicit = IsImplicit; + CXXThisExprBits.CapturedByCopyInLambdaWithExplicitObjectParameter = false; CXXThisExprBits.Loc = L; setDependence(computeDependence(this)); } @@ -1170,6 +1171,15 @@ class CXXThisExpr : public Expr { bool isImplicit() const { return CXXThisExprBits.IsImplicit; } void setImplicit(bool I) { CXXThisExprBits.IsImplicit = I; } + bool isCapturedByCopyInLambdaWithExplicitObjectParameter() const { + return CXXThisExprBits.CapturedByCopyInLambdaWithExplicitObjectParameter; + } + + void setCapturedByCopyInLambdaWithExplicitObjectParameter(bool Set) { + CXXThisExprBits.CapturedByCopyInLambdaWithExplicitObjectParameter = Set; + setDependence(computeDependence(this)); + } + static bool classof(const Stmt *T) { return T->getStmtClass() == CXXThisExprClass; } diff --git a/clang/include/clang/AST/Stmt.h b/clang/include/clang/AST/Stmt.h index 8892518d58e85..1b9c923104771 100644 --- a/clang/include/clang/AST/Stmt.h +++ b/clang/include/clang/AST/Stmt.h @@ -784,6 +784,11 @@ class alignas(void *) Stmt { LLVM_PREFERRED_TYPE(bool) unsigned IsImplicit : 1; + /// Whether there is a lambda with an explicit object parameter that + /// captures this "this" by copy. + LLVM_PREFERRED_TYPE(bool) + unsigned CapturedByCopyInLambdaWithExplicitObjectParameter : 1; + /// The location of the "this". SourceLocation Loc; }; diff --git a/clang/lib/AST/ComputeDependence.cpp b/clang/lib/AST/ComputeDependence.cpp index 86b77b49a0fbc..5ec3013fabba9 100644 --- a/clang/lib/AST/ComputeDependence.cpp +++ b/clang/lib/AST/ComputeDependence.cpp @@ -310,6 +310,16 @@ ExprDependence clang::computeDependence(CXXThisExpr *E) { // 'this' is type-dependent if the class type of the enclosing // member function is dependent (C++ [temp.dep.expr]p2) auto D = toExprDependenceForImpliedType(E->getType()->getDependence()); + + // If a lambda with an explicit object parameter captures '*this', then + // 'this' now refers to the captured copy of lambda, and if the lambda + // is type-dependent, so is the object and thus 'this'. + // + // Note: The standard does not mention this case explicitly, but we need + // to do this so we can mark NSDM accesses as dependent. + if (E->isCapturedByCopyInLambdaWithExplicitObjectParameter()) + D |= ExprDependence::Type; + assert(!(D & ExprDependence::UnexpandedPack)); return D; } diff --git a/clang/lib/AST/StmtProfile.cpp b/clang/lib/AST/StmtProfile.cpp index d68547f444c52..bec8bc71f5554 100644 --- a/clang/lib/AST/StmtProfile.cpp +++ b/clang/lib/AST/StmtProfile.cpp @@ -2011,6 +2011,7 @@ void StmtProfiler::VisitMSPropertySubscriptExpr( void StmtProfiler::VisitCXXThisExpr(const CXXThisExpr *S) { VisitExpr(S); ID.AddBoolean(S->isImplicit()); + ID.AddBoolean(S->isCapturedByCopyInLambdaWithExplicitObjectParameter()); } void StmtProfiler::VisitCXXThrowExpr(const CXXThrowExpr *S) { diff --git a/clang/lib/AST/TextNodeDumper.cpp b/clang/lib/AST/TextNodeDumper.cpp index f498de6374348..431f5d8bdb2b5 100644 --- a/clang/lib/AST/TextNodeDumper.cpp +++ b/clang/lib/AST/TextNodeDumper.cpp @@ -1194,8 +1194,11 @@ void TextNodeDumper::VisitDeclRefExpr(const DeclRefExpr *Node) { case NOUR_Constant: OS << " non_odr_use_constant"; break; case NOUR_Discarded: OS << " non_odr_use_discarded"; break; } - if (Node->refersToEnclosingVariableOrCapture()) + if (Node->isCapturedByCopyInLambdaWithExplicitObjectParameter()) + OS << " dependent_capture"; + else if (Node->refersToEnclosingVariableOrCapture()) OS << " refers_to_enclosing_variable_or_capture"; + if (Node->isImmediateEscalating()) OS << " immediate-escalating"; } @@ -1351,6 +1354,8 @@ void TextNodeDumper::VisitCXXBoolLiteralExpr(const CXXBoolLiteralExpr *Node) { void TextNodeDumper::VisitCXXThisExpr(const CXXThisExpr *Node) { if (Node->isImplicit()) OS << " implicit"; + if (Node->isCapturedByCopyInLambdaWithExplicitObjectParameter()) + OS << " dependent_capture"; OS << " this"; } diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index 7b91bbe0b2054..bd221763b319d 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -20720,20 +20720,42 @@ void Sema::MarkVariableReferenced(SourceLocation Loc, VarDecl *Var) { static void FixDependencyOfIdExpressionsInLambdaWithDependentObjectParameter( Sema &SemaRef, ValueDecl *D, Expr *E) { auto *ID = dyn_cast(E); - if (!ID || ID->isTypeDependent()) + if (!ID || ID->isTypeDependent() || !ID->refersToEnclosingVariableOrCapture()) return; + // If any enclosing lambda with a dependent explicit object parameter either + // explicitly captures the variable by value, or has a capture default of '=' + // and does not capture the variable by reference, then the type of the DRE + // is dependent on the type of that lambda's explicit object parameter. auto IsDependent = [&]() { - const LambdaScopeInfo *LSI = SemaRef.getCurLambda(); - if (!LSI) - return false; - if (!LSI->ExplicitObjectParameter || - !LSI->ExplicitObjectParameter->getType()->isDependentType()) - return false; - if (!LSI->CaptureMap.count(D)) - return false; - const Capture &Cap = LSI->getCapture(D); - return !Cap.isCopyCapture(); + for (auto *Scope : llvm::reverse(SemaRef.FunctionScopes)) { + auto *LSI = dyn_cast(Scope); + if (!LSI) + continue; + + if (LSI->Lambda && !LSI->Lambda->Encloses(SemaRef.CurContext) && + LSI->AfterParameterList) + return false; + + const auto *MD = LSI->CallOperator; + if (MD->getType().isNull()) + continue; + + const auto *Ty = cast(MD->getType()); + if (!Ty || !MD->isExplicitObjectMemberFunction() || + !Ty->getParamType(0)->isDependentType()) + continue; + + if (auto *C = LSI->CaptureMap.count(D) ? &LSI->getCapture(D) : nullptr) { + if (C->isCopyCapture()) + return true; + continue; + } + + if (LSI->ImpCaptureStyle == LambdaScopeInfo::ImpCap_LambdaByval) + return true; + } + return false; }(); ID->setCapturedByCopyInLambdaWithExplicitObjectParameter( diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp index 9ba4143240196..80c01b14379c9 100644 --- a/clang/lib/Sema/SemaExprCXX.cpp +++ b/clang/lib/Sema/SemaExprCXX.cpp @@ -1462,6 +1462,42 @@ Expr *Sema::BuildCXXThisExpr(SourceLocation Loc, QualType Type, void Sema::MarkThisReferenced(CXXThisExpr *This) { CheckCXXThisCapture(This->getExprLoc()); + if (This->isTypeDependent()) + return; + + // Check if 'this' is captured by value in a lambda with a dependent explicit + // object parameter, and mark it as type-dependent as well if so. + auto IsDependent = [&]() { + for (auto *Scope : llvm::reverse(FunctionScopes)) { + auto *LSI = dyn_cast(Scope); + if (!LSI) + continue; + + if (LSI->Lambda && !LSI->Lambda->Encloses(CurContext) && + LSI->AfterParameterList) + return false; + + // If this lambda captures 'this' by value, then 'this' is dependent iff + // this lambda has a dependent explicit object parameter. If we can't + // determine whether it does (e.g. because the CXXMethodDecl's type is + // null), assume it doesn't. + if (LSI->isCXXThisCaptured()) { + if (!LSI->getCXXThisCapture().isCopyCapture()) + continue; + + const auto *MD = LSI->CallOperator; + if (MD->getType().isNull()) + return false; + + const auto *Ty = cast(MD->getType()); + return Ty && MD->isExplicitObjectMemberFunction() && + Ty->getParamType(0)->isDependentType(); + } + } + return false; + }(); + + This->setCapturedByCopyInLambdaWithExplicitObjectParameter(IsDependent); } bool Sema::isThisOutsideMemberFunctionBody(QualType BaseType) { diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h index 6ebca07eaa4f9..13d7b00430d52 100644 --- a/clang/lib/Sema/TreeTransform.h +++ b/clang/lib/Sema/TreeTransform.h @@ -11178,8 +11178,8 @@ TreeTransform::TransformDeclRefExpr(DeclRefExpr *E) { } if (!getDerived().AlwaysRebuild() && - QualifierLoc == E->getQualifierLoc() && - ND == E->getDecl() && + !E->isCapturedByCopyInLambdaWithExplicitObjectParameter() && + QualifierLoc == E->getQualifierLoc() && ND == E->getDecl() && Found == E->getFoundDecl() && NameInfo.getName() == E->getDecl()->getDeclName() && !E->hasExplicitTemplateArgs()) { @@ -12642,9 +12642,17 @@ TreeTransform::TransformCXXThisExpr(CXXThisExpr *E) { // // In other contexts, the type of `this` may be overrided // for type deduction, so we need to recompute it. - QualType T = getSema().getCurLambda() ? - getDerived().TransformType(E->getType()) - : getSema().getCurrentThisType(); + // + // Always recompute the type if we're in the body of a lambda, and + // 'this' is dependent on a lambda's explicit object parameter. + QualType T = [&]() { + auto &S = getSema(); + if (E->isCapturedByCopyInLambdaWithExplicitObjectParameter()) + return S.getCurrentThisType(); + if (S.getCurLambda()) + return getDerived().TransformType(E->getType()); + return S.getCurrentThisType(); + }(); if (!getDerived().AlwaysRebuild() && T == E->getType()) { // Mark it referenced in the new context regardless. diff --git a/clang/lib/Serialization/ASTReaderStmt.cpp b/clang/lib/Serialization/ASTReaderStmt.cpp index f0984c3e46960..3ddb15b10f48b 100644 --- a/clang/lib/Serialization/ASTReaderStmt.cpp +++ b/clang/lib/Serialization/ASTReaderStmt.cpp @@ -1841,6 +1841,7 @@ void ASTStmtReader::VisitCXXThisExpr(CXXThisExpr *E) { VisitExpr(E); E->setLocation(readSourceLocation()); E->setImplicit(Record.readInt()); + E->setCapturedByCopyInLambdaWithExplicitObjectParameter(Record.readInt()); } void ASTStmtReader::VisitCXXThrowExpr(CXXThrowExpr *E) { diff --git a/clang/lib/Serialization/ASTWriterStmt.cpp b/clang/lib/Serialization/ASTWriterStmt.cpp index 0651614e2ce54..ddee5db2b69ef 100644 --- a/clang/lib/Serialization/ASTWriterStmt.cpp +++ b/clang/lib/Serialization/ASTWriterStmt.cpp @@ -1839,6 +1839,7 @@ void ASTStmtWriter::VisitCXXThisExpr(CXXThisExpr *E) { VisitExpr(E); Record.AddSourceLocation(E->getLocation()); Record.push_back(E->isImplicit()); + Record.push_back(E->isCapturedByCopyInLambdaWithExplicitObjectParameter()); Code = serialization::EXPR_CXX_THIS; } diff --git a/clang/test/CodeGenCXX/cxx2b-deducing-this.cpp b/clang/test/CodeGenCXX/cxx2b-deducing-this.cpp index de8c124c050eb..b755e80db35a1 100644 --- a/clang/test/CodeGenCXX/cxx2b-deducing-this.cpp +++ b/clang/test/CodeGenCXX/cxx2b-deducing-this.cpp @@ -109,3 +109,76 @@ void test_temporary() { //CHECK: %ref.tmp = alloca %struct.MaterializedTemporary, align 1 //CHECK: call void @_ZN21MaterializedTemporaryC1Ev(ptr noundef nonnull align 1 dereferenceable(1) %ref.tmp){{.*}} //CHECK invoke void @_ZNH21MaterializedTemporary3fooEOS_(ptr noundef nonnull align 1 dereferenceable(1) %ref.tmp){{.*}} + +namespace GH86399 { +volatile int a = 0; +struct function { + function& operator=(function const&) { + a = 1; + return *this; + } +}; + +void f() { + function list; + + //CHECK-LABEL: define internal void @"_ZZN7GH863991f{{.*}}"(ptr %{{.*}}) + //CHECK: call {{.*}} @_ZN7GH863998functionaSERKS0_ + //CHECK-NEXT: ret void + [&list](this auto self) { + list = function{}; + }(); +} +} + +namespace GH84163 { +// Just check that this doesn't crash (we were previously not instantiating +// everything that needs instantiating in here). +template struct S {}; + +void a() { + int x; + const auto l = [&x](this auto&) { S q; }; + l(); +} +} + +namespace GH84425 { +// As above. +void do_thing(int x) { + auto second = [&](this auto const& self, int b) -> int { + if (x) return x; + else return self(x); + }; + + second(1); +} + +void do_thing2(int x) { + auto second = [&](this auto const& self) { + if (true) return x; + else return x; + }; + + second(); +} +} + +namespace GH79754 { +// As above. +void f() { + int x; + [&x](this auto&&) {return x;}(); +} +} + +namespace GH70604 { +auto dothing(int num) +{ + auto fun = [&num](this auto&& self) -> void { + auto copy = num; + }; + + fun(); +} +} diff --git a/clang/test/PCH/cxx23-deducing-this-lambda.cpp b/clang/test/PCH/cxx23-deducing-this-lambda.cpp new file mode 100644 index 0000000000000..21b4bf0d633f2 --- /dev/null +++ b/clang/test/PCH/cxx23-deducing-this-lambda.cpp @@ -0,0 +1,35 @@ +// RUN: %clang_cc1 -emit-pch -std=c++23 -o %t %s +// RUN: %clang_cc1 -include-pch %t -verify -fsyntax-only -DTEST -std=c++23 %s + +// Test that dependence of 'this' and DREs due to by-value capture by a +// lambda with an explicit object parameter is serialised/deserialised +// properly. + +#ifndef HEADER +#define HEADER +struct S { + int x; + auto f() { + return [*this] (this auto&&) { + int y; + x = 42; + + const auto l = [y] (this auto&&) { y = 42; }; + l(); + }; + } +}; +#endif + +// expected-error@* {{read-only variable is not assignable}} +// expected-error@* {{cannot assign to a variable captured by copy in a non-mutable lambda}} +// expected-note@* 2 {{in instantiation of}} + +#ifdef TEST +void f() { + const auto l = S{}.f(); + l(); // expected-note {{in instantiation of}} +} +#endif + + diff --git a/clang/test/SemaCXX/cxx2b-deducing-this.cpp b/clang/test/SemaCXX/cxx2b-deducing-this.cpp index b8ddb9ad30003..c14c971afd235 100644 --- a/clang/test/SemaCXX/cxx2b-deducing-this.cpp +++ b/clang/test/SemaCXX/cxx2b-deducing-this.cpp @@ -200,6 +200,118 @@ void TestMutationInLambda() { [i = 0](this auto){ i++; }(); [i = 0](this const auto&){ i++; }(); // expected-error@-1 {{cannot assign to a variable captured by copy in a non-mutable lambda}} + // expected-note@-2 {{in instantiation of}} + + int x; + const auto l1 = [x](this auto&) { x = 42; }; // expected-error {{cannot assign to a variable captured by copy in a non-mutable lambda}} + const auto l2 = [=](this auto&) { x = 42; }; // expected-error {{cannot assign to a variable captured by copy in a non-mutable lambda}} + + const auto l3 = [&x](this auto&) { + const auto l3a = [x](this auto&) { x = 42; }; // expected-error {{cannot assign to a variable captured by copy in a non-mutable lambda}} + l3a(); // expected-note {{in instantiation of}} + }; + + const auto l4 = [&x](this auto&) { + const auto l4a = [=](this auto&) { x = 42; }; // expected-error {{cannot assign to a variable captured by copy in a non-mutable lambda}} + l4a(); // expected-note {{in instantiation of}} + }; + + const auto l5 = [x](this auto&) { + const auto l5a = [x](this auto&) { x = 42; }; // expected-error {{cannot assign to a variable captured by copy in a non-mutable lambda}} + l5a(); // expected-note {{in instantiation of}} + }; + + const auto l6 = [=](this auto&) { + const auto l6a = [=](this auto&) { x = 42; }; // expected-error {{cannot assign to a variable captured by copy in a non-mutable lambda}} + l6a(); // expected-note {{in instantiation of}} + }; + + const auto l7 = [x](this auto&) { + const auto l7a = [=](this auto&) { x = 42; }; // expected-error {{cannot assign to a variable captured by copy in a non-mutable lambda}} + l7a(); // expected-note {{in instantiation of}} + }; + + const auto l8 = [=](this auto&) { + const auto l8a = [x](this auto&) { x = 42; }; // expected-error {{cannot assign to a variable captured by copy in a non-mutable lambda}} + l8a(); // expected-note {{in instantiation of}} + }; + + const auto l9 = [&](this auto&) { + const auto l9a = [x](this auto&) { x = 42; }; // expected-error {{cannot assign to a variable captured by copy in a non-mutable lambda}} + l9a(); // expected-note {{in instantiation of}} + }; + + const auto l10 = [&](this auto&) { + const auto l10a = [=](this auto&) { x = 42; }; // expected-error {{cannot assign to a variable captured by copy in a non-mutable lambda}} + l10a(); // expected-note {{in instantiation of}} + }; + + const auto l11 = [x](this auto&) { + const auto l11a = [&x](this auto&) { x = 42; }; // expected-error {{cannot assign to a variable captured by copy in a non-mutable lambda}} expected-note {{while substituting}} + l11a(); + }; + + const auto l12 = [x](this auto&) { + const auto l12a = [&](this auto&) { x = 42; }; // expected-error {{cannot assign to a variable captured by copy in a non-mutable lambda}} expected-note {{while substituting}} + l12a(); + }; + + const auto l13 = [=](this auto&) { + const auto l13a = [&x](this auto&) { x = 42; }; // expected-error {{cannot assign to a variable captured by copy in a non-mutable lambda}} expected-note {{while substituting}} + l13a(); + }; + + struct S { + int x; + auto f() { + return [*this] (this auto&&) { + x = 42; // expected-error {{read-only variable is not assignable}} + [*this] () mutable { x = 42; } (); + [*this] (this auto&&) { x = 42; } (); + [*this] () { x = 42; } (); // expected-error {{read-only variable is not assignable}} + const auto l = [*this] (this auto&&) { x = 42; }; // expected-error {{read-only variable is not assignable}} + l(); // expected-note {{in instantiation of}} + + struct T { + int x; + auto g() { + return [&] (this auto&&) { + x = 42; + const auto l = [*this] (this auto&&) { x = 42; }; // expected-error {{read-only variable is not assignable}} + l(); // expected-note {{in instantiation of}} + }; + } + }; + + const auto l2 = T{}.g(); + l2(); // expected-note {{in instantiation of}} + }; + } + }; + + const auto l14 = S{}.f(); + + l1(); // expected-note {{in instantiation of}} + l2(); // expected-note {{in instantiation of}} + l3(); // expected-note {{in instantiation of}} + l4(); // expected-note {{in instantiation of}} + l5(); // expected-note {{in instantiation of}} + l6(); // expected-note {{in instantiation of}} + l7(); // expected-note {{in instantiation of}} + l8(); // expected-note {{in instantiation of}} + l9(); // expected-note {{in instantiation of}} + l10(); // expected-note {{in instantiation of}} + l11(); // expected-note {{in instantiation of}} + l12(); // expected-note {{in instantiation of}} + l13(); // expected-note {{in instantiation of}} + l14(); // expected-note 3 {{in instantiation of}} + + { + const auto l1 = [&x](this auto&) { x = 42; }; + const auto l2 = [&](this auto&) { x = 42; }; + l1(); + l2(); + } } struct Over_Call_Func_Example { @@ -650,3 +762,67 @@ int bug() { S{}.f(0); } } + +namespace GH84163 { +struct S { + int x; + + auto foo() { + return [*this](this auto&&) { + x = 10; // expected-error {{read-only variable is not assignable}} + }; + } +}; + +int f() { + S s{ 5 }; + const auto l = s.foo(); + l(); // expected-note {{in instantiation of}} + + const auto g = [x = 10](this auto&& self) { x = 20; }; // expected-error {{cannot assign to a variable captured by copy in a non-mutable lambda}} + g(); // expected-note {{in instantiation of}} +} +} + +namespace GH86054 { +template +struct unique_lock { + unique_lock(M&) {} +}; +int f() { + struct mutex {} cursor_guard; + [&cursor_guard](this auto self) { + unique_lock a(cursor_guard); + }(); +} +} + +namespace GH86398 { +struct function {}; // expected-note 2 {{not viable}} +int f() { + function list; + [&list](this auto self) { + list = self; // expected-error {{no viable overloaded '='}} + }(); // expected-note {{in instantiation of}} +} + +struct function2 { + function2& operator=(function2 const&) = delete; // expected-note {{candidate function not viable}} +}; +int g() { + function2 list; + [&list](this auto self) { + list = self; // expected-error {{no viable overloaded '='}} + }(); // expected-note {{in instantiation of}} +} + +struct function3 { + function3& operator=(function3 const&) = delete; // expected-note {{has been explicitly deleted}} +}; +int h() { + function3 list; + [&list](this auto self) { + list = function3{}; // expected-error {{selected deleted operator '='}} + }(); +} +}