https://github.com/Sirraide updated https://github.com/llvm/llvm-project/pull/89828
>From b5422012a65165f27bb31be7e9490892f663acfe Mon Sep 17 00:00:00 2001 From: Sirraide <aeternalm...@gmail.com> Date: Tue, 23 Apr 2024 22:45:29 +0200 Subject: [PATCH 1/3] [Clang] [CodeGen] Perform derived-to-base conversion on explicit object parameter in lambda --- clang/docs/ReleaseNotes.rst | 3 + clang/lib/CodeGen/CGExpr.cpp | 23 +++++++ clang/test/CodeGenCXX/cxx2b-deducing-this.cpp | 63 +++++++++++++++++++ 3 files changed, 89 insertions(+) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index d1f7293a842bb6..34aad4abf39619 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -562,6 +562,9 @@ Bug Fixes to C++ Support - Fixed a crash when trying to evaluate a user-defined ``static_assert`` message whose ``size()`` function returns a large or negative value. Fixes (#GH89407). - Fixed a use-after-free bug in parsing of type constraints with default arguments that involve lambdas. (#GH67235) +- 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/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp index 931cb391342ea2..33795d7d4d1921 100644 --- a/clang/lib/CodeGen/CGExpr.cpp +++ b/clang/lib/CodeGen/CGExpr.cpp @@ -4684,6 +4684,29 @@ 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<CXXRecordDecl>(Field->getParent()); + if (ThisTy != LambdaTy) { + CXXBasePaths Paths(/*FindAmbiguities=*/false, /*RecordPaths=*/true, + /*DetectVirtual=*/false); + + [[maybe_unused]] bool Derived = ThisTy->isDerivedFrom(LambdaTy, Paths); + assert(Derived && "Type not derived from lambda type?"); + + const CXXBasePath *Path = &Paths.front(); + CXXCastPath BasePathArray; + for (unsigned I = 0, E = Path->size(); I != E; ++I) + BasePathArray.push_back( + const_cast<CXXBaseSpecifier *>((*Path)[I].Base)); + + 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/test/CodeGenCXX/cxx2b-deducing-this.cpp b/clang/test/CodeGenCXX/cxx2b-deducing-this.cpp index b755e80db35a12..649fe2afbf4e91 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 <typename... Ts> +struct Overloaded : Ts... { + using Ts::operator()...; +}; + +template <typename... Ts> +Overloaded(Ts...) -> Overloaded<Ts...>; + +// 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(); +} +} >From 90d73ea88016307532bb38c4b2e8fa8f082bea75 Mon Sep 17 00:00:00 2001 From: Sirraide <aeternalm...@gmail.com> Date: Thu, 25 Apr 2024 01:01:18 +0200 Subject: [PATCH 2/3] [Clang] Tentative implementation of CWG 2881 --- clang/include/clang/AST/ASTContext.h | 9 +++ .../clang/Basic/DiagnosticSemaKinds.td | 5 ++ clang/include/clang/Sema/Sema.h | 4 +- clang/lib/CodeGen/CGExpr.cpp | 17 +---- clang/lib/Sema/SemaLambda.cpp | 61 ++++++++++++---- clang/lib/Sema/SemaOverload.cpp | 15 ++-- clang/test/CXX/drs/dr28xx.cpp | 71 +++++++++++++++++++ 7 files changed, 147 insertions(+), 35 deletions(-) diff --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h index d5ed20ff50157d..3210ef2dfe12be 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<CXXBaseSpecifier *, 4> CXXCastPath; + namespace Builtin { class Context; @@ -1168,6 +1171,12 @@ class ASTContext : public RefCountedBase<ASTContext> { /// in device compilation. llvm::DenseSet<const FunctionDecl *> 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<const CXXMethodDecl *, CXXCastPath> 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 63e951daec7477..5e04ec82ea152b 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -7497,6 +7497,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 1ca523ec88c2f9..fa450a7868282a 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -7144,7 +7144,9 @@ class Sema final : public SemaBase { StorageClass SC, ArrayRef<ParmVarDecl *> 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 33795d7d4d1921..73f8a67c10fe82 100644 --- a/clang/lib/CodeGen/CGExpr.cpp +++ b/clang/lib/CodeGen/CGExpr.cpp @@ -4667,7 +4667,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<CXXMethodDecl>(CurCodeDecl)) { + const auto *MD = dyn_cast_if_present<CXXMethodDecl>(CurCodeDecl); + if (MD) { HasExplicitObjectParameter = MD->isExplicitObjectMemberFunction(); assert(MD->getParent()->isLambda()); assert(MD->getParent() == Field->getParent()); @@ -4689,22 +4690,10 @@ LValue CodeGenFunction::EmitLValueForLambdaField(const FieldDecl *Field, auto *ThisTy = D->getType().getNonReferenceType()->getAsCXXRecordDecl(); auto *LambdaTy = cast<CXXRecordDecl>(Field->getParent()); if (ThisTy != LambdaTy) { - CXXBasePaths Paths(/*FindAmbiguities=*/false, /*RecordPaths=*/true, - /*DetectVirtual=*/false); - - [[maybe_unused]] bool Derived = ThisTy->isDerivedFrom(LambdaTy, Paths); - assert(Derived && "Type not derived from lambda type?"); - - const CXXBasePath *Path = &Paths.front(); - CXXCastPath BasePathArray; - for (unsigned I = 0, E = Path->size(); I != E; ++I) - BasePathArray.push_back( - const_cast<CXXBaseSpecifier *>((*Path)[I].Base)); - + 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 { diff --git a/clang/lib/Sema/SemaLambda.cpp b/clang/lib/Sema/SemaLambda.cpp index 1743afaf15287f..c96f376d1e2bea 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,62 @@ 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? + 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 04cd9e78739d20..88782cf64c95df 100644 --- a/clang/lib/Sema/SemaOverload.cpp +++ b/clang/lib/Sema/SemaOverload.cpp @@ -6492,17 +6492,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<Expr *> &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 @@ -15671,8 +15674,10 @@ ExprResult Sema::BuildCallToMemberFunction(Scope *S, Expr *MemExprE, CallExpr *TheCall = nullptr; llvm::SmallVector<Expr *, 8> 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, @@ -15986,9 +15991,7 @@ Sema::BuildCallToObjectOfClassType(Scope *S, Expr *Obj, // Initialize the object parameter. llvm::SmallVector<Expr *, 8> 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/dr28xx.cpp b/clang/test/CXX/drs/dr28xx.cpp index 4d9b0c76758d53..ef140a91b9c494 100644 --- a/clang/test/CXX/drs/dr28xx.cpp +++ b/clang/test/CXX/drs/dr28xx.cpp @@ -81,3 +81,74 @@ struct A { #endif } // namespace cwg2858 + +namespace cwg2881 { // cwg2881: 19 + +#if __cplusplus >= 202302L + +template <typename T> struct A : T {}; +template <typename T> struct B : T {}; +template <typename T> struct C : virtual T { C(T t) : T(t) {} }; +template <typename T> struct D : virtual T { D(T t) : T(t) {} }; + +template <typename Ts> +struct O1 : A<Ts>, B<Ts> { + using A<Ts>::operator(); + using B<Ts>::operator(); +}; + +template <typename Ts> struct O2 : protected Ts { // expected-note {{declared protected here}} + using Ts::operator(); + O2(Ts ts) : Ts(ts) {} +}; + +template <typename Ts> struct O3 : private Ts { // expected-note {{declared private here}} + using Ts::operator(); + O3(Ts ts) : Ts(ts) {} +}; + +// Not ambiguous because of virtual inheritance. +template <typename Ts> +struct O4 : C<Ts>, D<Ts> { + using C<Ts>::operator(); + using D<Ts>::operator(); + O4(Ts t) : Ts(t), C<Ts>(t), D<Ts>(t) {} +}; + +// This still has a public path to the lambda, and it's also not +// ambiguous because of virtual inheritance. +template <typename Ts> +struct O5 : private C<Ts>, D<Ts> { + using C<Ts>::operator(); + using D<Ts>::operator(); + O5(Ts t) : Ts(t), C<Ts>(t), D<Ts>(t) {} +}; + +// This is only invalid if we call T's call operator. +template <typename T, typename U> +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<decltype(L1)>{L1, L1}(); // expected-error {{inaccessible due to ambiguity}} + O1<decltype(L2)>{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 + >From a6d81ad1e5b2ed58cb920ef7d6112276466bff4c Mon Sep 17 00:00:00 2001 From: Sirraide <aeternalm...@gmail.com> Date: Mon, 29 Apr 2024 09:49:19 +0200 Subject: [PATCH 3/3] [NFC] Add date to dr test --- clang/test/CXX/drs/dr28xx.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/test/CXX/drs/dr28xx.cpp b/clang/test/CXX/drs/dr28xx.cpp index ef140a91b9c494..57c0c7312047d3 100644 --- a/clang/test/CXX/drs/dr28xx.cpp +++ b/clang/test/CXX/drs/dr28xx.cpp @@ -82,7 +82,7 @@ struct A { } // namespace cwg2858 -namespace cwg2881 { // cwg2881: 19 +namespace cwg2881 { // cwg2881: 19 open 2024-04-19 #if __cplusplus >= 202302L _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits