[clang] 0370beb - [Clang] Perform derived-to-base conversion on explicit object parameter in lambda (#89828)
via cfe-commits
cfe-commits at lists.llvm.org
Wed May 22 11:15:50 PDT 2024
Author: Sirraide
Date: 2024-05-22T20:15:44+02:00
New Revision: 0370beb230a35f00b7d07c50ab95f8777662a0c6
URL: https://github.com/llvm/llvm-project/commit/0370beb230a35f00b7d07c50ab95f8777662a0c6
DIFF: https://github.com/llvm/llvm-project/commit/0370beb230a35f00b7d07c50ab95f8777662a0c6.diff
LOG: [Clang] Perform derived-to-base conversion on explicit object parameter in lambda (#89828)
Consider this code:
```c++
template <typename... Ts>
struct Overloaded : Ts... { using Ts::operator()...; };
template <typename... Ts>
Overloaded(Ts...) -> Overloaded<Ts...>;
void f() {
int x;
Overloaded o {
[&](this auto& self) {
return &x;
}
};
o();
}
```
To access `x` in the lambda, we need to perform derived-to-base
conversion on `self` (since the type of `self` is not the lambda type,
but rather `Overloaded<(lambda type)>`). We were previously missing this
step, causing us to attempt to load the entire lambda (as the base
class, it would end up being the ‘field’ with index `0` here), which
would then assert later on in codegen.
Moreover, this is only valid in the first place if there is a unique and
publicly accessible cast path from the derived class to the lambda’s
type, so this also adds a check in Sema to diagnose problematic
cases.
This fixes #87210 and fixes #89541.
Added:
Modified:
clang/docs/ReleaseNotes.rst
clang/include/clang/AST/ASTContext.h
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Sema/Sema.h
clang/lib/CodeGen/CGExpr.cpp
clang/lib/Sema/SemaLambda.cpp
clang/lib/Sema/SemaOverload.cpp
clang/test/CXX/drs/cwg28xx.cpp
clang/test/CodeGenCXX/cxx2b-deducing-this.cpp
clang/www/cxx_dr_status.html
Removed:
################################################################################
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 2b35e2162ab5b..0c4a343b70009 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -766,6 +766,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<CXXBaseSpecifier *, 4> CXXCastPath;
+
namespace Builtin {
class Context;
@@ -1170,6 +1173,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 34531411f1c0f..761243ed96cb7 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<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 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<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());
@@ -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<CXXRecordDecl>(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<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
@@ -15612,8 +15615,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,
@@ -15927,9 +15932,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/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 <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
+
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 <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();
+}
+}
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 @@ <h2 id="cxxdr">C++ defect report implementation status</h2>
<td><a href="https://cplusplus.github.io/CWG/issues/2881.html">2881</a></td>
<td>tentatively ready</td>
<td>Type restrictions for the explicit object parameter of a lambda</td>
- <td align="center">Not resolved</td>
+ <td title="Clang 19 implements 2024-04-19 resolution" align="center">Not Resolved*</td>
</tr>
<tr class="open" id="2882">
<td><a href="https://cplusplus.github.io/CWG/issues/2882.html">2882</a></td>
More information about the cfe-commits
mailing list