[clang] __is_trivially_equality_comparable for types containing lambdas

[AMP999]

Lambdas (closure types) are trivially equality-comparable iff they are
non-capturing, because non-capturing lambdas are convertible to function
pointers: if (lam1 == lam2) compiles, then lam1 and lam2 must have
the same type, and be always-equal, and be empty.

[llvmbot]

@llvm/pr-subscribers-clang-codegen

@llvm/pr-subscribers-clang

Changes

Lambdas (closure types) are trivially equality-comparable iff they are
non-capturing, because non-capturing lambdas are convertible to function
pointers: if (lam1 == lam2) compiles, then lam1 and lam2 must have
the same type, and be always-equal, and be empty.

---

Full diff: https://github.com/llvm/llvm-project/pull/68506.diff

2 Files Affected:

• (modified) clang/lib/AST/Type.cpp (+2)
• (modified) clang/test/SemaCXX/type-traits.cpp (+21-3)

diff --git a/clang/lib/AST/Type.cpp b/clang/lib/AST/Type.cpp
index 4c433f7fe9daca0..f0e419de3b1ee84 100644
--- a/clang/lib/AST/Type.cpp
+++ b/clang/lib/AST/Type.cpp
@@ -2663,6 +2663,8 @@ static bool
 HasNonDeletedDefaultedEqualityComparison(const CXXRecordDecl *Decl) {
   if (Decl->isUnion())
     return false;
+  if (Decl->isLambda())
+    return Decl->captures().empty() && (Decl->getLambdaCaptureDefault() == LCD_None);
 
   auto IsDefaultedOperatorEqualEqual = [&](const FunctionDecl *Function) {
     return Function->getOverloadedOperator() ==
diff --git a/clang/test/SemaCXX/type-traits.cpp b/clang/test/SemaCXX/type-traits.cpp
index a35689d52978fcc..275ddcbae73930d 100644
--- a/clang/test/SemaCXX/type-traits.cpp
+++ b/clang/test/SemaCXX/type-traits.cpp
@@ -3160,11 +3160,18 @@ static_assert(!__is_trivially_equality_comparable(float), "");
 static_assert(!__is_trivially_equality_comparable(double), "");
 static_assert(!__is_trivially_equality_comparable(long double), "");
 
-struct TriviallyEqualityComparableNoDefaultedComparator {
+struct NonTriviallyEqualityComparableNoComparator {
   int i;
   int j;
 };
-static_assert(!__is_trivially_equality_comparable(TriviallyEqualityComparableNoDefaultedComparator), "");
+static_assert(!__is_trivially_equality_comparable(NonTriviallyEqualityComparableNoComparator), "");
+
+struct NonTriviallyEqualityComparableNonDefaultedComparator {
+  int i;
+  int j;
+  bool operator==(const NonTriviallyEqualityComparableNonDefaultedComparator&);
+};
+static_assert(!__is_trivially_equality_comparable(NonTriviallyEqualityComparableNonDefaultedComparator), "");
 
 #if __cplusplus >= 202002L
 
@@ -3177,7 +3184,7 @@ struct TriviallyEqualityComparable {
 
   bool operator==(const TriviallyEqualityComparable&) const = default;
 };
-static_assert(__is_trivially_equality_comparable(TriviallyEqualityComparable), "");
+static_assert(__is_trivially_equality_comparable(TriviallyEqualityComparable));
 
 struct TriviallyEqualityComparableContainsArray {
   int a[4];
@@ -3193,6 +3200,17 @@ struct TriviallyEqualityComparableContainsMultiDimensionArray {
 };
 static_assert(__is_trivially_equality_comparable(TriviallyEqualityComparableContainsMultiDimensionArray));
 
+auto GetNonCapturingLambda() { return [](){ return 42; }; }
+
+struct TriviallyEqualityComparableContainsLambda {
+  [[no_unique_address]] decltype(GetNonCapturingLambda()) l;
+  int i;
+
+  bool operator==(const TriviallyEqualityComparableContainsLambda&) const = default;
+};
+static_assert(!__is_trivially_equality_comparable(decltype(GetNonCapturingLambda()))); // padding
+static_assert(__is_trivially_equality_comparable(TriviallyEqualityComparableContainsLambda));
+
 struct TriviallyEqualityComparableNonTriviallyCopyable {
   TriviallyEqualityComparableNonTriviallyCopyable(const TriviallyEqualityComparableNonTriviallyCopyable&);
   ~TriviallyEqualityComparableNonTriviallyCopyable();

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[cor3ntin]

I think there is an opportunity for a small refactor here, but feel free to land this and we can handle the refactor later as an NFC change if you prefer

[cor3ntin]

This pattern is used in a few places, maybe we should factor it out

[AMP999]

I think there is an opportunity for a small refactor here, but feel free to land this and we can handle the refactor later as an NFC change if you prefer

Is this (344455e) what you had in mind? I only found a few places that could use the factored-out function, and in the "CodeGenFunction.cpp" case, I'm not even sure it applies. It looks like that codepath is intended to be hit when we codegen the operator() of a lambda with captures under -fsanitize=null, but I couldn't see any null-check in the generated code even when the lambda did have captures, so I'm not sure I understand it. That codepath is
@zygoloid's from 2017 (376c28e). Should I keep that diff, or revert it?

[cor3ntin]

Yup, this is what i have in mind. Afaik, the CodeGenFunction.cpp change was a bug that you fixed - just going off the comment.

[AMP999]

@cor3ntin Can you land this for me?

[efriedma-quic]

Do you need to check if the lambda is templated? For example, the following doesn't compile:

auto GetNonCapturingLambda() { return []<class T>(){ return 42; }; }
auto f() { return GetNonCapturingLambda() == GetNonCapturingLambda(); }