Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "[Clang] Fix __is_trivially_equality_comparable returning true with ineligebile defaulted overloads" #97002

Merged
merged 1 commit into from
Jun 28, 2024

Conversation

ZequanWu
Copy link
Contributor

@ZequanWu ZequanWu commented Jun 28, 2024

Reverts #93113:

Reason for reverting:
This causes failure on LLVM CI builder: #93113 (comment) and clang crashes on chromium build.

@ZequanWu ZequanWu requested a review from philnik777 June 28, 2024 04:36
@ZequanWu ZequanWu merged commit 567b2c6 into main Jun 28, 2024
5 of 7 checks passed
@ZequanWu ZequanWu deleted the revert-93113-fix_is_trivially_equality_comparable branch June 28, 2024 04:36
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Jun 28, 2024
@llvmbot
Copy link
Member

llvmbot commented Jun 28, 2024

@llvm/pr-subscribers-clang

Author: Zequan Wu (ZequanWu)

Changes

Reverts llvm/llvm-project#93113


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

5 Files Affected:

  • (modified) clang/docs/ReleaseNotes.rst (+1-4)
  • (modified) clang/include/clang/AST/Type.h (+3)
  • (modified) clang/lib/AST/Type.cpp (+60)
  • (modified) clang/lib/Sema/SemaExprCXX.cpp (+1-77)
  • (modified) clang/test/SemaCXX/type-traits.cpp (-43)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 7ebfc87144269..da967fcdda808 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -104,7 +104,7 @@ ABI Changes in This Version
   ifuncs. Its purpose was to preserve backwards compatibility when the ".ifunc"
   suffix got removed from the name mangling. The alias interacts badly with
   GlobalOpt (see the issue #96197).
-
+  
 - Fixed Microsoft name mangling for auto non-type template arguments of pointer
   type for MSVC 1920+. This change resolves incompatibilities with code compiled
   by MSVC 1920+ but will introduce incompatibilities with code compiled by
@@ -740,9 +740,6 @@ Bug Fixes in This Version
   negatives where the analysis failed to detect unchecked access to guarded
   data.
 
-- ``__is_trivially_equality_comparable`` no longer returns true for types which
-  have a constrained defaulted comparison operator (#GH89293).
-
 Bug Fixes to Compiler Builtins
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
diff --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h
index a98899f7f4222..62836ec5c6312 100644
--- a/clang/include/clang/AST/Type.h
+++ b/clang/include/clang/AST/Type.h
@@ -1142,6 +1142,9 @@ class QualType {
   /// Return true if this is a trivially relocatable type.
   bool isTriviallyRelocatableType(const ASTContext &Context) const;
 
+  /// Return true if this is a trivially equality comparable type.
+  bool isTriviallyEqualityComparableType(const ASTContext &Context) const;
+
   /// Returns true if it is a class and it might be dynamic.
   bool mayBeDynamicClass() const;
 
diff --git a/clang/lib/AST/Type.cpp b/clang/lib/AST/Type.cpp
index cc535aba4936e..d8b885870de3a 100644
--- a/clang/lib/AST/Type.cpp
+++ b/clang/lib/AST/Type.cpp
@@ -2815,6 +2815,66 @@ bool QualType::isTriviallyRelocatableType(const ASTContext &Context) const {
   }
 }
 
+static bool
+HasNonDeletedDefaultedEqualityComparison(const CXXRecordDecl *Decl) {
+  if (Decl->isUnion())
+    return false;
+  if (Decl->isLambda())
+    return Decl->isCapturelessLambda();
+
+  auto IsDefaultedOperatorEqualEqual = [&](const FunctionDecl *Function) {
+    return Function->getOverloadedOperator() ==
+               OverloadedOperatorKind::OO_EqualEqual &&
+           Function->isDefaulted() && Function->getNumParams() > 0 &&
+           (Function->getParamDecl(0)->getType()->isReferenceType() ||
+            Decl->isTriviallyCopyable());
+  };
+
+  if (llvm::none_of(Decl->methods(), IsDefaultedOperatorEqualEqual) &&
+      llvm::none_of(Decl->friends(), [&](const FriendDecl *Friend) {
+        if (NamedDecl *ND = Friend->getFriendDecl()) {
+          return ND->isFunctionOrFunctionTemplate() &&
+                 IsDefaultedOperatorEqualEqual(ND->getAsFunction());
+        }
+        return false;
+      }))
+    return false;
+
+  return llvm::all_of(Decl->bases(),
+                      [](const CXXBaseSpecifier &BS) {
+                        if (const auto *RD = BS.getType()->getAsCXXRecordDecl())
+                          return HasNonDeletedDefaultedEqualityComparison(RD);
+                        return true;
+                      }) &&
+         llvm::all_of(Decl->fields(), [](const FieldDecl *FD) {
+           auto Type = FD->getType();
+           if (Type->isArrayType())
+             Type = Type->getBaseElementTypeUnsafe()->getCanonicalTypeUnqualified();
+
+           if (Type->isReferenceType() || Type->isEnumeralType())
+             return false;
+           if (const auto *RD = Type->getAsCXXRecordDecl())
+             return HasNonDeletedDefaultedEqualityComparison(RD);
+           return true;
+         });
+}
+
+bool QualType::isTriviallyEqualityComparableType(
+    const ASTContext &Context) const {
+  QualType CanonicalType = getCanonicalType();
+  if (CanonicalType->isIncompleteType() || CanonicalType->isDependentType() ||
+      CanonicalType->isEnumeralType() || CanonicalType->isArrayType())
+    return false;
+
+  if (const auto *RD = CanonicalType->getAsCXXRecordDecl()) {
+    if (!HasNonDeletedDefaultedEqualityComparison(RD))
+      return false;
+  }
+
+  return Context.hasUniqueObjectRepresentations(
+      CanonicalType, /*CheckIfTriviallyCopyable=*/false);
+}
+
 bool QualType::isNonWeakInMRRWithObjCWeak(const ASTContext &Context) const {
   return !Context.getLangOpts().ObjCAutoRefCount &&
          Context.getLangOpts().ObjCWeak &&
diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index db2cec0708900..f3af8dee6b090 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -5201,82 +5201,6 @@ static bool HasNoThrowOperator(const RecordType *RT, OverloadedOperatorKind Op,
   return false;
 }
 
-static bool
-HasNonDeletedDefaultedEqualityComparison(Sema &S, const CXXRecordDecl *Decl) {
-  if (Decl->isUnion())
-    return false;
-  if (Decl->isLambda())
-    return Decl->isCapturelessLambda();
-
-  {
-    EnterExpressionEvaluationContext UnevaluatedContext(
-        S, Sema::ExpressionEvaluationContext::Unevaluated);
-    Sema::SFINAETrap SFINAE(S, /*AccessCheckingSFINAE=*/true);
-    Sema::ContextRAII TUContext(S, S.Context.getTranslationUnitDecl());
-
-    // const ClassT& obj;
-    OpaqueValueExpr Operand(
-        {}, Decl->getTypeForDecl()->getCanonicalTypeUnqualified().withConst(),
-        ExprValueKind::VK_LValue);
-    UnresolvedSet<16> Functions;
-    // obj == obj;
-    S.LookupBinOp(S.TUScope, {}, BinaryOperatorKind::BO_EQ, Functions);
-
-    auto Result = S.CreateOverloadedBinOp({}, BinaryOperatorKind::BO_EQ,
-                                          Functions, &Operand, &Operand);
-    if (Result.isInvalid() || SFINAE.hasErrorOccurred())
-      return false;
-
-    const auto *CallExpr = dyn_cast<CXXOperatorCallExpr>(Result.get());
-    if (!CallExpr)
-      return false;
-    const auto *Callee = CallExpr->getDirectCallee();
-    auto ParamT = Callee->getParamDecl(0)->getType();
-    if (!Callee->isDefaulted())
-      return false;
-    if (!ParamT->isReferenceType() && !Decl->isTriviallyCopyable())
-      return false;
-    if (ParamT.getNonReferenceType()->getUnqualifiedDesugaredType() !=
-        Decl->getTypeForDecl())
-      return false;
-  }
-
-  return llvm::all_of(Decl->bases(),
-                      [&](const CXXBaseSpecifier &BS) {
-                        if (const auto *RD = BS.getType()->getAsCXXRecordDecl())
-                          return HasNonDeletedDefaultedEqualityComparison(S,
-                                                                          RD);
-                        return true;
-                      }) &&
-         llvm::all_of(Decl->fields(), [&](const FieldDecl *FD) {
-           auto Type = FD->getType();
-           if (Type->isArrayType())
-             Type = Type->getBaseElementTypeUnsafe()
-                        ->getCanonicalTypeUnqualified();
-
-           if (Type->isReferenceType() || Type->isEnumeralType())
-             return false;
-           if (const auto *RD = Type->getAsCXXRecordDecl())
-             return HasNonDeletedDefaultedEqualityComparison(S, RD);
-           return true;
-         });
-}
-
-static bool isTriviallyEqualityComparableType(Sema &S, QualType Type) {
-  QualType CanonicalType = Type.getCanonicalType();
-  if (CanonicalType->isIncompleteType() || CanonicalType->isDependentType() ||
-      CanonicalType->isEnumeralType() || CanonicalType->isArrayType())
-    return false;
-
-  if (const auto *RD = CanonicalType->getAsCXXRecordDecl()) {
-    if (!HasNonDeletedDefaultedEqualityComparison(S, RD))
-      return false;
-  }
-
-  return S.getASTContext().hasUniqueObjectRepresentations(
-      CanonicalType, /*CheckIfTriviallyCopyable=*/false);
-}
-
 static bool EvaluateUnaryTypeTrait(Sema &Self, TypeTrait UTT,
                                    SourceLocation KeyLoc,
                                    TypeSourceInfo *TInfo) {
@@ -5709,7 +5633,7 @@ static bool EvaluateUnaryTypeTrait(Sema &Self, TypeTrait UTT,
     Self.Diag(KeyLoc, diag::err_builtin_pass_in_regs_non_class) << T;
     return false;
   case UTT_IsTriviallyEqualityComparable:
-    return isTriviallyEqualityComparableType(Self, T);
+    return T.isTriviallyEqualityComparableType(C);
   }
 }
 
diff --git a/clang/test/SemaCXX/type-traits.cpp b/clang/test/SemaCXX/type-traits.cpp
index ec2bc4c96044d..d40605f56f1ed 100644
--- a/clang/test/SemaCXX/type-traits.cpp
+++ b/clang/test/SemaCXX/type-traits.cpp
@@ -3677,12 +3677,6 @@ struct NonTriviallyEqualityComparableNoComparator {
 };
 static_assert(!__is_trivially_equality_comparable(NonTriviallyEqualityComparableNoComparator));
 
-struct NonTriviallyEqualityComparableConvertibleToBuiltin {
-  int i;
-  operator unsigned() const;
-};
-static_assert(!__is_trivially_equality_comparable(NonTriviallyEqualityComparableConvertibleToBuiltin));
-
 struct NonTriviallyEqualityComparableNonDefaultedComparator {
   int i;
   int j;
@@ -3891,45 +3885,8 @@ struct NotTriviallyEqualityComparableNonTriviallyEqualityComparableArrs2 {
 
   bool operator==(const NotTriviallyEqualityComparableNonTriviallyEqualityComparableArrs2&) const = default;
 };
-
 static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableNonTriviallyEqualityComparableArrs2));
 
-template<bool B>
-struct MaybeTriviallyEqualityComparable {
-    int i;
-    bool operator==(const MaybeTriviallyEqualityComparable&) const requires B = default;
-    bool operator==(const MaybeTriviallyEqualityComparable& rhs) const { return (i % 3) == (rhs.i % 3); }
-};
-static_assert(__is_trivially_equality_comparable(MaybeTriviallyEqualityComparable<true>));
-static_assert(!__is_trivially_equality_comparable(MaybeTriviallyEqualityComparable<false>));
-
-struct NotTriviallyEqualityComparableMoreConstrainedExternalOp {
-  int i;
-  bool operator==(const NotTriviallyEqualityComparableMoreConstrainedExternalOp&) const = default;
-};
-
-bool operator==(const NotTriviallyEqualityComparableMoreConstrainedExternalOp&,
-                const NotTriviallyEqualityComparableMoreConstrainedExternalOp&) __attribute__((enable_if(true, ""))) {}
-
-static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableMoreConstrainedExternalOp));
-
-struct TriviallyEqualityComparableExternalDefaultedOp {
-  int i;
-  friend bool operator==(TriviallyEqualityComparableExternalDefaultedOp, TriviallyEqualityComparableExternalDefaultedOp);
-};
-bool operator==(TriviallyEqualityComparableExternalDefaultedOp, TriviallyEqualityComparableExternalDefaultedOp) = default;
-
-static_assert(__is_trivially_equality_comparable(TriviallyEqualityComparableExternalDefaultedOp));
-
-struct EqualityComparableBase {
-  bool operator==(const EqualityComparableBase&) const = default;
-};
-
-struct ComparingBaseOnly : EqualityComparableBase {
-  int j_ = 0;
-};
-static_assert(!__is_trivially_equality_comparable(ComparingBaseOnly));
-
 namespace hidden_friend {
 
 struct TriviallyEqualityComparable {

lravenclaw pushed a commit to lravenclaw/llvm-project that referenced this pull request Jul 3, 2024
philnik777 added a commit to philnik777/llvm-project that referenced this pull request Jul 6, 2024
…e with ineligebile defaulted overloads" (llvm#97002)

This reverts commit 567b2c6.
philnik777 added a commit that referenced this pull request Jul 12, 2024
…e with ineligebile defaulted overloads" (#97002) (#97894)

This reverts commit 567b2c6.
philnik777 added a commit to philnik777/llvm-project that referenced this pull request Jul 14, 2024
…ning true with ineligebile defaulted overloads" (llvm#97002)"

This reverts commit 43b1972.
aaryanshukla pushed a commit to aaryanshukla/llvm-project that referenced this pull request Jul 14, 2024
…e with ineligebile defaulted overloads" (llvm#97002) (llvm#97894)

This reverts commit 567b2c6.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants