Skip to content

[Clang][SemaCXX] Preserve qualifiers in derived-to-base cast in defaulted comparison operators #102619

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

Merged

Conversation

MitalAshok
Copy link
Contributor

Fixes #102588

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Aug 9, 2024
@llvmbot
Copy link
Member

llvmbot commented Aug 9, 2024

@llvm/pr-subscribers-clang

Author: Mital Ashok (MitalAshok)

Changes

Fixes #102588


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

3 Files Affected:

  • (modified) clang/docs/ReleaseNotes.rst (+1)
  • (modified) clang/lib/Sema/SemaDeclCXX.cpp (+5-4)
  • (modified) clang/test/SemaCXX/cxx20-default-compare.cpp (+50)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 7a05ccf3184111..30bc36bd0d6cd7 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -216,6 +216,7 @@ Bug Fixes to C++ Support
 - Clang now preserves the unexpanded flag in a lambda transform used for pack expansion. (#GH56852), (#GH85667),
   (#GH99877).
 - Fixed a bug when diagnosing ambiguous explicit specializations of constrained member functions.
+- Fixed a bug where defaulted comparison operators would remove ``const`` from base classes. (#GH102588)
 
 Bug Fixes to AST Handling
 ^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index b07e555afcaccf..a478bdb46faa05 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -8414,10 +8414,11 @@ class DefaultedComparisonSynthesizer
     if (Obj.first.isInvalid() || Obj.second.isInvalid())
       return {ExprError(), ExprError()};
     CXXCastPath Path = {Base};
-    return {S.ImpCastExprToType(Obj.first.get(), Base->getType(),
-                                CK_DerivedToBase, VK_LValue, &Path),
-            S.ImpCastExprToType(Obj.second.get(), Base->getType(),
-                                CK_DerivedToBase, VK_LValue, &Path)};
+    const auto CastToBase = [&](Expr *E) {
+      QualType ToType = S.Context.getQualifiedType(Base->getType(), E->getType().getQualifiers());
+      return S.ImpCastExprToType(E, ToType, CK_DerivedToBase, VK_LValue, &Path);
+    };
+    return {CastToBase(Obj.first.get()), CastToBase(Obj.second.get())};
   }
 
   ExprPair getField(FieldDecl *Field) {
diff --git a/clang/test/SemaCXX/cxx20-default-compare.cpp b/clang/test/SemaCXX/cxx20-default-compare.cpp
index 7074ee885ac4a2..3e4673c31e4890 100644
--- a/clang/test/SemaCXX/cxx20-default-compare.cpp
+++ b/clang/test/SemaCXX/cxx20-default-compare.cpp
@@ -1,5 +1,7 @@
 // RUN: %clang_cc1 %s -std=c++23 -verify -Wfloat-equal
 
+#include "Inputs/std-compare.h"
+
 struct Foo {
   float val;
   bool operator==(const Foo &) const;
@@ -15,3 +17,51 @@ bool operator==(const Foo &, const Foo &) = default;  // expected-warning {{comp
 
 // Declare the defaulted comparison function as a non-member function. Arguments are passed by value.
 bool operator==(Foo, Foo) = default;  // expected-warning {{comparing floating point with == or != is unsafe}} expected-note {{in defaulted equality comparison operator for 'Foo' first required here}}
+
+namespace GH102588 {
+struct A {
+  int i = 0;
+  constexpr operator int() const { return i; }
+  constexpr operator int&() { return ++i; }
+};
+
+struct B : A {
+  bool operator==(const B &) const = default;
+};
+
+constexpr bool f() {
+  B x;
+  return x == x;
+}
+
+static_assert(f());
+
+struct ConstOnly {
+  std::strong_ordering operator<=>(const ConstOnly&) const;
+  std::strong_ordering operator<=>(ConstOnly&) = delete;
+  friend bool operator==(const ConstOnly&, const ConstOnly&);
+  friend bool operator==(ConstOnly&, ConstOnly&) = delete;
+};
+
+struct MutOnly {
+  std::strong_ordering operator<=>(const MutOnly&) const = delete;;
+  std::strong_ordering operator<=>(MutOnly&);
+  friend bool operator==(const MutOnly&, const MutOnly&) = delete;;
+  friend bool operator==(MutOnly&, MutOnly&);
+};
+
+struct ConstCheck : ConstOnly {
+  friend std::strong_ordering operator<=>(const ConstCheck&, const ConstCheck&) = default;
+  std::strong_ordering operator<=>(ConstCheck const& __restrict) const __restrict = default;
+  friend bool operator==(const ConstCheck&, const ConstCheck&) = default;
+  bool operator==(this const ConstCheck&, const ConstCheck&) = default;
+};
+
+// FIXME: Non-reference explicit object parameter are rejected
+struct MutCheck : MutOnly {
+  friend bool operator==(MutCheck, MutCheck) = default;
+  // std::strong_ordering operator<=>(this MutCheck, MutCheck) = default;
+  friend std::strong_ordering operator<=>(MutCheck, MutCheck) = default;
+  // bool operator==(this MutCheck, MutCheck) = default;
+};
+}

Copy link

github-actions bot commented Aug 9, 2024

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

@MitalAshok MitalAshok force-pushed the fix-synth-comp-derived-to-base-qualifiers branch from f473409 to fc43618 Compare August 9, 2024 14:10
Copy link
Contributor

@cor3ntin cor3ntin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@cor3ntin cor3ntin merged commit f1ac334 into llvm:main Sep 5, 2024
4 of 5 checks passed
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.

default equality operator== const selects not-const conversion operator
3 participants