diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 93b6ba59ecf9f..f646382acaff9 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -754,6 +754,9 @@ Bug Fixes to C++ Support - Clang now correctly diagnoses when the current instantiation is used as an incomplete base class. - Clang no longer treats ``constexpr`` class scope function template specializations of non-static members as implicitly ``const`` in language modes after C++11. +- Fixed a crash when trying to emit captures in a lambda call operator with an explicit object + parameter that is called on a derived type of the lambda. + Fixes (#GH87210), (GH89541). Bug Fixes to AST Handling ^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h index 2ce2b810d3636..a1d1d1c51cd41 100644 --- a/clang/include/clang/AST/ASTContext.h +++ b/clang/include/clang/AST/ASTContext.h @@ -110,6 +110,9 @@ class VarTemplateDecl; class VTableContextBase; class XRayFunctionFilter; +/// A simple array of base specifiers. +typedef SmallVector CXXCastPath; + namespace Builtin { class Context; @@ -1170,6 +1173,12 @@ class ASTContext : public RefCountedBase { /// in device compilation. llvm::DenseSet CUDAImplicitHostDeviceFunUsedByDevice; + /// For capturing lambdas with an explicit object parameter whose type is + /// derived from the lambda type, we need to perform derived-to-base + /// conversion so we can access the captures; the cast paths for that + /// are stored here. + llvm::DenseMap LambdaCastPaths; + ASTContext(LangOptions &LOpts, SourceManager &SM, IdentifierTable &idents, SelectorTable &sels, Builtin::Context &builtins, TranslationUnitKind TUKind); diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 41a9745ddb570..e00e595a762bd 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -7525,6 +7525,11 @@ def err_explicit_object_parameter_mutable: Error< def err_invalid_explicit_object_type_in_lambda: Error< "invalid explicit object parameter type %0 in lambda with capture; " "the type must be the same as, or derived from, the lambda">; +def err_explicit_object_lambda_ambiguous_base : Error< + "lambda %0 is inaccessible due to ambiguity:%1">; +def err_explicit_object_lambda_inaccessible_base : Error< + "invalid explicit object parameter type %0 in lambda with capture; " + "the type must derive publicly from the lambda">; def err_ref_qualifier_overload : Error< "cannot overload a member function %select{without a ref-qualifier|with " diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 4e628b21e65e3..97784f5ae0dc8 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -7071,7 +7071,9 @@ class Sema final : public SemaBase { StorageClass SC, ArrayRef Params, bool HasExplicitResultType); - void DiagnoseInvalidExplicitObjectParameterInLambda(CXXMethodDecl *Method); + /// Returns true if the explicit object parameter was invalid. + bool DiagnoseInvalidExplicitObjectParameterInLambda(CXXMethodDecl *Method, + SourceLocation CallLoc); /// Perform initialization analysis of the init-capture and perform /// any implicit conversions such as an lvalue-to-rvalue conversion if diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp index 6f9237e2067f5..4c4a42134dd4a 100644 --- a/clang/lib/CodeGen/CGExpr.cpp +++ b/clang/lib/CodeGen/CGExpr.cpp @@ -4676,7 +4676,8 @@ LValue CodeGenFunction::EmitMemberExpr(const MemberExpr *E) { LValue CodeGenFunction::EmitLValueForLambdaField(const FieldDecl *Field, llvm::Value *ThisValue) { bool HasExplicitObjectParameter = false; - if (const auto *MD = dyn_cast_if_present(CurCodeDecl)) { + const auto *MD = dyn_cast_if_present(CurCodeDecl); + if (MD) { HasExplicitObjectParameter = MD->isExplicitObjectMemberFunction(); assert(MD->getParent()->isLambda()); assert(MD->getParent() == Field->getParent()); @@ -4693,6 +4694,17 @@ LValue CodeGenFunction::EmitLValueForLambdaField(const FieldDecl *Field, else LambdaLV = MakeAddrLValue(AddrOfExplicitObject, D->getType().getNonReferenceType()); + + // Make sure we have an lvalue to the lambda itself and not a derived class. + auto *ThisTy = D->getType().getNonReferenceType()->getAsCXXRecordDecl(); + auto *LambdaTy = cast(Field->getParent()); + if (ThisTy != LambdaTy) { + const CXXCastPath &BasePathArray = getContext().LambdaCastPaths.at(MD); + Address Base = GetAddressOfBaseClass( + LambdaLV.getAddress(*this), ThisTy, BasePathArray.begin(), + BasePathArray.end(), /*NullCheckValue=*/false, SourceLocation()); + LambdaLV = MakeAddrLValue(Base, QualType{LambdaTy->getTypeForDecl(), 0}); + } } else { QualType LambdaTagType = getContext().getTagDeclType(Field->getParent()); LambdaLV = MakeNaturalAlignAddrLValue(ThisValue, LambdaTagType); diff --git a/clang/lib/Sema/SemaLambda.cpp b/clang/lib/Sema/SemaLambda.cpp index 1743afaf15287..276a43ad79b91 100644 --- a/clang/lib/Sema/SemaLambda.cpp +++ b/clang/lib/Sema/SemaLambda.cpp @@ -12,6 +12,7 @@ #include "clang/Sema/SemaLambda.h" #include "TypeLocBuilder.h" #include "clang/AST/ASTLambda.h" +#include "clang/AST/CXXInheritance.h" #include "clang/AST/ExprCXX.h" #include "clang/Basic/TargetInfo.h" #include "clang/Sema/DeclSpec.h" @@ -386,30 +387,69 @@ buildTypeForLambdaCallOperator(Sema &S, clang::CXXRecordDecl *Class, // parameter, if any, of the lambda's function call operator (possibly // instantiated from a function call operator template) shall be either: // - the closure type, -// - class type derived from the closure type, or +// - class type publicly and unambiguously derived from the closure type, or // - a reference to a possibly cv-qualified such type. -void Sema::DiagnoseInvalidExplicitObjectParameterInLambda( - CXXMethodDecl *Method) { +bool Sema::DiagnoseInvalidExplicitObjectParameterInLambda( + CXXMethodDecl *Method, SourceLocation CallLoc) { if (!isLambdaCallWithExplicitObjectParameter(Method)) - return; + return false; CXXRecordDecl *RD = Method->getParent(); if (Method->getType()->isDependentType()) - return; + return false; if (RD->isCapturelessLambda()) - return; - QualType ExplicitObjectParameterType = Method->getParamDecl(0) - ->getType() + return false; + + ParmVarDecl *Param = Method->getParamDecl(0); + QualType ExplicitObjectParameterType = Param->getType() .getNonReferenceType() .getUnqualifiedType() .getDesugaredType(getASTContext()); QualType LambdaType = getASTContext().getRecordType(RD); if (LambdaType == ExplicitObjectParameterType) - return; - if (IsDerivedFrom(RD->getLocation(), ExplicitObjectParameterType, LambdaType)) - return; - Diag(Method->getParamDecl(0)->getLocation(), - diag::err_invalid_explicit_object_type_in_lambda) - << ExplicitObjectParameterType; + return false; + + // Don't check the same instantiation twice. + // + // If this call operator is ill-formed, there is no point in issuing + // a diagnostic every time it is called because the problem is in the + // definition of the derived type, not at the call site. + // + // FIXME: Move this check to where we instantiate the method? This should + // be possible, but the naive approach of just marking the method as invalid + // leads to us emitting more diagnostics than we should have to for this case + // (1 error here *and* 1 error about there being no matching overload at the + // call site). It might be possible to avoid that by also checking if there + // is an empty cast path for the method stored in the context (signalling that + // we've already diagnosed it) and then just not building the call, but that + // doesn't really seem any simpler than diagnosing it at the call site... + if (auto It = Context.LambdaCastPaths.find(Method); + It != Context.LambdaCastPaths.end()) + return It->second.empty(); + + CXXCastPath &Path = Context.LambdaCastPaths[Method]; + CXXBasePaths Paths(/*FindAmbiguities=*/true, /*RecordPaths=*/true, + /*DetectVirtual=*/false); + if (!IsDerivedFrom(RD->getLocation(), ExplicitObjectParameterType, LambdaType, + Paths)) { + Diag(Param->getLocation(), diag::err_invalid_explicit_object_type_in_lambda) + << ExplicitObjectParameterType; + return true; + } + + if (Paths.isAmbiguous(LambdaType->getCanonicalTypeUnqualified())) { + std::string PathsDisplay = getAmbiguousPathsDisplayString(Paths); + Diag(CallLoc, diag::err_explicit_object_lambda_ambiguous_base) + << LambdaType << PathsDisplay; + return true; + } + + if (CheckBaseClassAccess(CallLoc, LambdaType, ExplicitObjectParameterType, + Paths.front(), + diag::err_explicit_object_lambda_inaccessible_base)) + return true; + + BuildBasePathArray(Paths, Path); + return false; } void Sema::handleLambdaNumbering( diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp index 2eb25237a0de6..0c89fca8d38eb 100644 --- a/clang/lib/Sema/SemaOverload.cpp +++ b/clang/lib/Sema/SemaOverload.cpp @@ -6472,17 +6472,20 @@ ExprResult Sema::InitializeExplicitObjectArgument(Sema &S, Expr *Obj, Obj->getExprLoc(), Obj); } -static void PrepareExplicitObjectArgument(Sema &S, CXXMethodDecl *Method, +static bool PrepareExplicitObjectArgument(Sema &S, CXXMethodDecl *Method, Expr *Object, MultiExprArg &Args, SmallVectorImpl &NewArgs) { assert(Method->isExplicitObjectMemberFunction() && "Method is not an explicit member function"); assert(NewArgs.empty() && "NewArgs should be empty"); + NewArgs.reserve(Args.size() + 1); Expr *This = GetExplicitObjectExpr(S, Object, Method); NewArgs.push_back(This); NewArgs.append(Args.begin(), Args.end()); Args = NewArgs; + return S.DiagnoseInvalidExplicitObjectParameterInLambda( + Method, Object->getBeginLoc()); } /// Determine whether the provided type is an integral type, or an enumeration @@ -15612,8 +15615,10 @@ ExprResult Sema::BuildCallToMemberFunction(Scope *S, Expr *MemExprE, CallExpr *TheCall = nullptr; llvm::SmallVector NewArgs; if (Method->isExplicitObjectMemberFunction()) { - PrepareExplicitObjectArgument(*this, Method, MemExpr->getBase(), Args, - NewArgs); + if (PrepareExplicitObjectArgument(*this, Method, MemExpr->getBase(), Args, + NewArgs)) + return ExprError(); + // Build the actual expression node. ExprResult FnExpr = CreateFunctionRefExpr(*this, Method, FoundDecl, MemExpr, @@ -15927,9 +15932,7 @@ Sema::BuildCallToObjectOfClassType(Scope *S, Expr *Obj, // Initialize the object parameter. llvm::SmallVector NewArgs; if (Method->isExplicitObjectMemberFunction()) { - // FIXME: we should do that during the definition of the lambda when we can. - DiagnoseInvalidExplicitObjectParameterInLambda(Method); - PrepareExplicitObjectArgument(*this, Method, Obj, Args, NewArgs); + IsError |= PrepareExplicitObjectArgument(*this, Method, Obj, Args, NewArgs); } else { ExprResult ObjRes = PerformImplicitObjectArgumentInitialization( Object.get(), /*Qualifier=*/nullptr, Best->FoundDecl, Method); diff --git a/clang/test/CXX/drs/cwg28xx.cpp b/clang/test/CXX/drs/cwg28xx.cpp index 696cd1b9c84e7..8469a065ccaa0 100644 --- a/clang/test/CXX/drs/cwg28xx.cpp +++ b/clang/test/CXX/drs/cwg28xx.cpp @@ -109,3 +109,74 @@ struct A { #endif } // namespace cwg2858 + +namespace cwg2881 { // cwg2881: 19 tentatively ready 2024-04-19 + +#if __cplusplus >= 202302L + +template struct A : T {}; +template struct B : T {}; +template struct C : virtual T { C(T t) : T(t) {} }; +template struct D : virtual T { D(T t) : T(t) {} }; + +template +struct O1 : A, B { + using A::operator(); + using B::operator(); +}; + +template struct O2 : protected Ts { // expected-note {{declared protected here}} + using Ts::operator(); + O2(Ts ts) : Ts(ts) {} +}; + +template struct O3 : private Ts { // expected-note {{declared private here}} + using Ts::operator(); + O3(Ts ts) : Ts(ts) {} +}; + +// Not ambiguous because of virtual inheritance. +template +struct O4 : C, D { + using C::operator(); + using D::operator(); + O4(Ts t) : Ts(t), C(t), D(t) {} +}; + +// This still has a public path to the lambda, and it's also not +// ambiguous because of virtual inheritance. +template +struct O5 : private C, D { + using C::operator(); + using D::operator(); + O5(Ts t) : Ts(t), C(t), D(t) {} +}; + +// This is only invalid if we call T's call operator. +template +struct O6 : private T, U { // expected-note {{declared private here}} + using T::operator(); + using U::operator(); + O6(T t, U u) : T(t), U(u) {} +}; + +void f() { + int x; + auto L1 = [=](this auto&& self) { (void) &x; }; + auto L2 = [&](this auto&& self) { (void) &x; }; + O1{L1, L1}(); // expected-error {{inaccessible due to ambiguity}} + O1{L2, L2}(); // expected-error {{inaccessible due to ambiguity}} + O2{L1}(); // expected-error {{must derive publicly from the lambda}} + O3{L1}(); // expected-error {{must derive publicly from the lambda}} + O4{L1}(); + O5{L1}(); + O6 o{L1, L2}; + o.decltype(L1)::operator()(); // expected-error {{must derive publicly from the lambda}} + o.decltype(L1)::operator()(); // No error here because we've already diagnosed this method. + o.decltype(L2)::operator()(); +} + +#endif + +} // namespace cwg2881 + diff --git a/clang/test/CodeGenCXX/cxx2b-deducing-this.cpp b/clang/test/CodeGenCXX/cxx2b-deducing-this.cpp index b755e80db35a1..649fe2afbf4e9 100644 --- a/clang/test/CodeGenCXX/cxx2b-deducing-this.cpp +++ b/clang/test/CodeGenCXX/cxx2b-deducing-this.cpp @@ -182,3 +182,66 @@ auto dothing(int num) fun(); } } + +namespace GH87210 { +template +struct Overloaded : Ts... { + using Ts::operator()...; +}; + +template +Overloaded(Ts...) -> Overloaded; + +// CHECK-LABEL: define dso_local void @_ZN7GH872101fEv() +// CHECK-NEXT: entry: +// CHECK-NEXT: [[X:%.*]] = alloca i32 +// CHECK-NEXT: [[Over:%.*]] = alloca %"{{.*}}Overloaded" +// CHECK: call noundef ptr @"_ZZN7GH872101fEvENH3$_0clINS_10OverloadedIJS0_EEEEEDaRT_"(ptr {{.*}} [[Over]]) +void f() { + int x; + Overloaded o { + // CHECK: define internal noundef ptr @"_ZZN7GH872101fEvENH3$_0clINS_10OverloadedIJS0_EEEEEDaRT_"(ptr {{.*}} [[Self:%.*]]) + // CHECK-NEXT: entry: + // CHECK-NEXT: [[SelfAddr:%.*]] = alloca ptr + // CHECK-NEXT: store ptr [[Self]], ptr [[SelfAddr]] + // CHECK-NEXT: [[SelfPtr:%.*]] = load ptr, ptr [[SelfAddr]] + // CHECK-NEXT: [[XRef:%.*]] = getelementptr inbounds %{{.*}}, ptr [[SelfPtr]], i32 0, i32 0 + // CHECK-NEXT: [[X:%.*]] = load ptr, ptr [[XRef]] + // CHECK-NEXT: ret ptr [[X]] + [&](this auto& self) { + return &x; + } + }; + o(); +} + +void g() { + int x; + Overloaded o { + [=](this auto& self) { + return x; + } + }; + o(); +} +} + +namespace GH89541 { +// Same as above; just check that this doesn't crash. +int one = 1; +auto factory(int& x = one) { + return [&](this auto self) { + x; + }; +}; + +using Base = decltype(factory()); +struct Derived : Base { + Derived() : Base(factory()) {} +}; + +void f() { + Derived d; + d(); +} +} diff --git a/clang/www/cxx_dr_status.html b/clang/www/cxx_dr_status.html index 9d458330f5376..8d9b83aa29bf9 100755 --- a/clang/www/cxx_dr_status.html +++ b/clang/www/cxx_dr_status.html @@ -17095,7 +17095,7 @@

C++ defect report implementation status

2881 tentatively ready Type restrictions for the explicit object parameter of a lambda - Not resolved + Not Resolved* 2882