[clang] e6e6e34 - [c++20] Defaulted comparison support for array members.

Richard Smith via cfe-commits cfe-commits at lists.llvm.org
Mon Dec 9 14:54:21 PST 2019


Author: Richard Smith
Date: 2019-12-09T14:54:06-08:00
New Revision: e6e6e34b95cfe03275943fde0db259cc7d57f4ad

URL: https://github.com/llvm/llvm-project/commit/e6e6e34b95cfe03275943fde0db259cc7d57f4ad
DIFF: https://github.com/llvm/llvm-project/commit/e6e6e34b95cfe03275943fde0db259cc7d57f4ad.diff

LOG: [c++20] Defaulted comparison support for array members.

Added: 
    

Modified: 
    clang/lib/Sema/SemaDeclCXX.cpp
    clang/test/CXX/class/class.compare/class.compare.default/p5.cpp
    clang/test/CXX/class/class.compare/class.eq/p2.cpp
    clang/test/CXX/class/class.compare/class.eq/p3.cpp
    clang/test/CXX/class/class.compare/class.spaceship/p1.cpp
    clang/test/CXX/class/class.compare/class.spaceship/p3.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 28159b9c0812..1ee6ee5dcf12 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -7442,6 +7442,7 @@ class DefaultedComparisonSynthesizer
                                         StmtListResult, StmtResult,
                                         std::pair<ExprResult, ExprResult>> {
   SourceLocation Loc;
+  unsigned ArrayDepth = 0;
 
 public:
   using Base = DefaultedComparisonVisitor;
@@ -7467,29 +7468,56 @@ class DefaultedComparisonSynthesizer
     case DefaultedComparisonKind::None:
       llvm_unreachable("not a defaulted comparison");
 
-    case DefaultedComparisonKind::Equal:
+    case DefaultedComparisonKind::Equal: {
       // C++2a [class.eq]p3:
       //   [...] compar[e] the corresponding elements [...] until the first
       //   index i where xi == yi yields [...] false. If no such index exists,
       //   V is true. Otherwise, V is false.
       //
       // Join the comparisons with '&&'s and return the result. Use a right
-      // fold because that short-circuits more naturally.
-      for (Stmt *EAsStmt : llvm::reverse(Stmts.Stmts)) {
-        Expr *E = cast<Expr>(EAsStmt);
-        if (RetVal.isUnset()) {
-          RetVal = E;
+      // fold (traversing the conditions right-to-left), because that
+      // short-circuits more naturally.
+      auto OldStmts = std::move(Stmts.Stmts);
+      Stmts.Stmts.clear();
+      ExprResult CmpSoFar;
+      // Finish a particular comparison chain.
+      auto FinishCmp = [&] {
+        if (Expr *Prior = CmpSoFar.get()) {
+          // Convert the last expression to 'return ...;'
+          if (RetVal.isUnset() && Stmts.Stmts.empty())
+            RetVal = CmpSoFar;
+          // Convert any prior comparison to 'if (!(...)) return false;'
+          else if (Stmts.add(buildIfNotCondReturnFalse(Prior)))
+            return true;
+          CmpSoFar = ExprResult();
+        }
+        return false;
+      };
+      for (Stmt *EAsStmt : llvm::reverse(OldStmts)) {
+        Expr *E = dyn_cast<Expr>(EAsStmt);
+        if (!E) {
+          // Found an array comparison.
+          if (FinishCmp() || Stmts.add(EAsStmt))
+            return StmtError();
+          continue;
+        }
+
+        if (CmpSoFar.isUnset()) {
+          CmpSoFar = E;
           continue;
         }
-        RetVal = S.CreateBuiltinBinOp(Loc, BO_LAnd, E, RetVal.get());
-        if (RetVal.isInvalid())
+        CmpSoFar = S.CreateBuiltinBinOp(Loc, BO_LAnd, E, CmpSoFar.get());
+        if (CmpSoFar.isInvalid())
           return StmtError();
       }
+      if (FinishCmp())
+        return StmtError();
+      std::reverse(Stmts.Stmts.begin(), Stmts.Stmts.end());
       //   If no such index exists, V is true.
       if (RetVal.isUnset())
         RetVal = S.ActOnCXXBoolLiteral(Loc, tok::kw_true);
-      Stmts.Stmts.clear();
       break;
+    }
 
     case DefaultedComparisonKind::ThreeWay: {
       // Per C++2a [class.spaceship]p3, as a fallback add:
@@ -7579,7 +7607,100 @@ class DefaultedComparisonSynthesizer
   // FIXME: When expanding a subobject, register a note in the code synthesis
   // stack to say which subobject we're comparing.
 
-  // FIXME: Build a loop for an array subobject.
+  StmtResult buildIfNotCondReturnFalse(ExprResult Cond) {
+    if (Cond.isInvalid())
+      return StmtError();
+
+    ExprResult NotCond = S.CreateBuiltinUnaryOp(Loc, UO_LNot, Cond.get());
+    if (NotCond.isInvalid())
+      return StmtError();
+
+    ExprResult False = S.ActOnCXXBoolLiteral(Loc, tok::kw_false);
+    assert(!False.isInvalid() && "should never fail");
+    StmtResult ReturnFalse = S.BuildReturnStmt(Loc, False.get());
+    if (ReturnFalse.isInvalid())
+      return StmtError();
+
+    return S.ActOnIfStmt(Loc, false, nullptr,
+                         S.ActOnCondition(nullptr, Loc, NotCond.get(),
+                                          Sema::ConditionKind::Boolean),
+                         ReturnFalse.get(), SourceLocation(), nullptr);
+  }
+
+  StmtResult visitSubobjectArray(QualType Type, llvm::APInt Size,
+                                 ExprPair Subobj) {
+    QualType SizeType = S.Context.getSizeType();
+    Size = Size.zextOrTrunc(S.Context.getTypeSize(SizeType));
+
+    // Build 'size_t i$n = 0'.
+    IdentifierInfo *IterationVarName = nullptr;
+    {
+      SmallString<8> Str;
+      llvm::raw_svector_ostream OS(Str);
+      OS << "i" << ArrayDepth;
+      IterationVarName = &S.Context.Idents.get(OS.str());
+    }
+    VarDecl *IterationVar = VarDecl::Create(
+        S.Context, S.CurContext, Loc, Loc, IterationVarName, SizeType,
+        S.Context.getTrivialTypeSourceInfo(SizeType, Loc), SC_None);
+    llvm::APInt Zero(S.Context.getTypeSize(SizeType), 0);
+    IterationVar->setInit(
+        IntegerLiteral::Create(S.Context, Zero, SizeType, Loc));
+    Stmt *Init = new (S.Context) DeclStmt(DeclGroupRef(IterationVar), Loc, Loc);
+
+    auto IterRef = [&] {
+      ExprResult Ref = S.BuildDeclarationNameExpr(
+          CXXScopeSpec(), DeclarationNameInfo(IterationVarName, Loc),
+          IterationVar);
+      assert(!Ref.isInvalid() && "can't reference our own variable?");
+      return Ref.get();
+    };
+
+    // Build 'i$n != Size'.
+    ExprResult Cond = S.CreateBuiltinBinOp(
+        Loc, BO_NE, IterRef(),
+        IntegerLiteral::Create(S.Context, Size, SizeType, Loc));
+    assert(!Cond.isInvalid() && "should never fail");
+
+    // Build '++i$n'.
+    ExprResult Inc = S.CreateBuiltinUnaryOp(Loc, UO_PreInc, IterRef());
+    assert(!Inc.isInvalid() && "should never fail");
+
+    // Build 'a[i$n]' and 'b[i$n]'.
+    auto Index = [&](ExprResult E) {
+      if (E.isInvalid())
+        return ExprError();
+      return S.CreateBuiltinArraySubscriptExpr(E.get(), Loc, IterRef(), Loc);
+    };
+    Subobj.first = Index(Subobj.first);
+    Subobj.second = Index(Subobj.second);
+
+    // Compare the array elements.
+    ++ArrayDepth;
+    StmtResult Substmt = visitSubobject(Type, Subobj);
+    --ArrayDepth;
+
+    if (Substmt.isInvalid())
+      return StmtError();
+
+    // For the inner level of an 'operator==', build 'if (!cmp) return false;'.
+    // For outer levels or for an 'operator<=>' we already have a suitable
+    // statement that returns as necessary.
+    if (Expr *ElemCmp = dyn_cast<Expr>(Substmt.get())) {
+      assert(DCK == DefaultedComparisonKind::Equal &&
+             "should have non-expression statement");
+      Substmt = buildIfNotCondReturnFalse(ElemCmp);
+      if (Substmt.isInvalid())
+        return StmtError();
+    }
+
+    // Build 'for (...) ...'
+    return S.ActOnForStmt(Loc, Loc, Init,
+                          S.ActOnCondition(nullptr, Loc, Cond.get(),
+                                           Sema::ConditionKind::Boolean),
+                          S.MakeFullDiscardedValueExpr(Inc.get()), Loc,
+                          Substmt.get());
+  }
 
   StmtResult visitExpandedSubobject(QualType Type, ExprPair Obj) {
     UnresolvedSet<4> Fns; // FIXME: Track this.

diff  --git a/clang/test/CXX/class/class.compare/class.compare.default/p5.cpp b/clang/test/CXX/class/class.compare/class.compare.default/p5.cpp
index f863ed09bc63..5489051a29c9 100644
--- a/clang/test/CXX/class/class.compare/class.compare.default/p5.cpp
+++ b/clang/test/CXX/class/class.compare/class.compare.default/p5.cpp
@@ -12,7 +12,7 @@ namespace std {
 
 // Check that we compare subobjects in the right order.
 struct Log {
-  char buff[8] = {};
+  char buff[10] = {};
   int n = 0;
   constexpr void add(char c) { buff[n++] = c; }
   constexpr bool operator==(const char *p) const { return __builtin_strcmp(p, buff) == 0; }
@@ -24,22 +24,37 @@ template<char C> struct B {
   constexpr std::strong_ordering operator<=>(const B&) const { log->add(C); return {0}; }
 };
 
+template<typename T> constexpr bool check(bool which, const char *str) {
+  Log log;
+  T c(&log);
+  (void)(which ? c == c : c <=> c);
+  return log == str;
+}
+
 struct C : B<'a'>, B<'b'> {
+  B<'r'> r[3];
   B<'c'> c;
+  B<'s'> s[2];
   B<'d'> d;
-  // FIXME: Test arrays once we handle them properly.
 
-  constexpr C(Log *p) : B<'a'>{p}, B<'b'>{p}, c{p}, d{p} {}
+  constexpr C(Log *p) : B<'a'>{p}, B<'b'>{p}, r{p, p, p}, c{p}, s{p, p}, d{p} {}
 
   bool operator==(const C&) const = default;
   std::strong_ordering operator<=>(const C&) const = default;
 };
 
-constexpr bool check(bool which) {
-  Log log;
-  C c(&log);
-  (void)(which ? c == c : c <=> c);
-  return log == "abcd";
-}
-static_assert(check(false));
-static_assert(check(true));
+static_assert(check<C>(false, "abrrrcssd"));
+static_assert(check<C>(true, "abrrrcssd"));
+
+struct D {
+  B<'x'> x;
+  B<'y'> y[2];
+
+  constexpr D(Log *p) : x{p}, y{p, p} {}
+
+  bool operator==(const D&) const = default;
+  std::strong_ordering operator<=>(const D&) const = default;
+};
+
+static_assert(check<D>(false, "xyy"));
+static_assert(check<D>(true, "xyy"));

diff  --git a/clang/test/CXX/class/class.compare/class.eq/p2.cpp b/clang/test/CXX/class/class.compare/class.eq/p2.cpp
index d53d071f36fe..55b2cc3c08f6 100644
--- a/clang/test/CXX/class/class.compare/class.eq/p2.cpp
+++ b/clang/test/CXX/class/class.compare/class.eq/p2.cpp
@@ -17,8 +17,8 @@ struct G { bool operator==(G) const = delete; }; // expected-note {{deleted here
 
 template<typename T> struct X {
   X();
-  bool operator==(const X&) const = default; // #x expected-note 3{{deleted here}}
-  T t; // expected-note 2{{because there is no viable comparison function for member 't'}}
+  bool operator==(const X&) const = default; // #x expected-note 4{{deleted here}}
+  T t; // expected-note 3{{because there is no viable comparison function for member 't'}}
        // expected-note at -1 {{because it would invoke a deleted comparison function for member 't'}}
 };
 
@@ -43,4 +43,7 @@ void test() {
   void(X<F>() == X<F>()); // expected-note {{in defaulted equality comparison operator for 'X<F>' first required here}}
 
   void(X<G>() == X<G>()); // expected-error {{cannot be compared because its 'operator==' is implicitly deleted}}
+
+  void(X<A[3]>() == X<A[3]>()); // expected-error {{cannot be compared because its 'operator==' is implicitly deleted}}
+  void(X<B[3]>() == X<B[3]>());
 }

diff  --git a/clang/test/CXX/class/class.compare/class.eq/p3.cpp b/clang/test/CXX/class/class.compare/class.eq/p3.cpp
index f58b9daa5235..a7356b5381cd 100644
--- a/clang/test/CXX/class/class.compare/class.eq/p3.cpp
+++ b/clang/test/CXX/class/class.compare/class.eq/p3.cpp
@@ -1,11 +1,13 @@
 // RUN: %clang_cc1 -std=c++2a -verify %s
 
 struct A {
-  int a, b, c;
+  int a, b[3], c;
   bool operator==(const A&) const = default;
 };
 
-static_assert(A{1, 2, 3} == A{1, 2, 3});
-static_assert(A{1, 2, 3} == A{0, 2, 3}); // expected-error {{failed}}
-static_assert(A{1, 2, 3} == A{1, 0, 3}); // expected-error {{failed}}
-static_assert(A{1, 2, 3} == A{1, 2, 0}); // expected-error {{failed}}
+static_assert(A{1, 2, 3, 4, 5} == A{1, 2, 3, 4, 5});
+static_assert(A{1, 2, 3, 4, 5} == A{0, 2, 3, 4, 5}); // expected-error {{failed}}
+static_assert(A{1, 2, 3, 4, 5} == A{1, 0, 3, 4, 5}); // expected-error {{failed}}
+static_assert(A{1, 2, 3, 4, 5} == A{1, 2, 0, 4, 5}); // expected-error {{failed}}
+static_assert(A{1, 2, 3, 4, 5} == A{1, 2, 3, 0, 5}); // expected-error {{failed}}
+static_assert(A{1, 2, 3, 4, 5} == A{1, 2, 3, 4, 0}); // expected-error {{failed}}

diff  --git a/clang/test/CXX/class/class.compare/class.spaceship/p1.cpp b/clang/test/CXX/class/class.compare/class.spaceship/p1.cpp
index fafa99ff5cfe..a131842d3944 100644
--- a/clang/test/CXX/class/class.compare/class.spaceship/p1.cpp
+++ b/clang/test/CXX/class/class.compare/class.spaceship/p1.cpp
@@ -1,7 +1,7 @@
 // RUN: %clang_cc1 -std=c++2a -verify %s -fcxx-exceptions
 
 namespace std {
-  struct strong_ordering { // expected-note 3{{candidate}}
+  struct strong_ordering { // expected-note 6{{candidate}}
     int n;
     constexpr operator int() const { return n; }
     static const strong_ordering less, equal, greater;
@@ -45,25 +45,25 @@ namespace Deletedness {
     bool operator<(const B&) const;
   };
   struct C {
-    std::strong_ordering operator<=>(const C&) const = delete; // expected-note {{deleted}}
+    std::strong_ordering operator<=>(const C&) const = delete; // expected-note 2{{deleted}}
   };
   struct D1 {
     bool operator==(const D1&) const;
-    std::strong_ordering operator<=>(int) const; // expected-note {{function not viable}} expected-note {{function (with reversed parameter order) not viable}}
-    bool operator<(int) const; // expected-note {{function not viable}}
+    std::strong_ordering operator<=>(int) const; // expected-note 2{{function not viable}} expected-note 2{{function (with reversed parameter order) not viable}}
+    bool operator<(int) const; // expected-note 2{{function not viable}}
   };
   struct D2 {
     bool operator<(const D2&) const;
-    std::strong_ordering operator<=>(int) const; // expected-note {{function not viable}} expected-note {{function (with reversed parameter order) not viable}}
-    bool operator==(int) const; // expected-note {{function not viable}}
+    std::strong_ordering operator<=>(int) const; // expected-note 2{{function not viable}} expected-note 2{{function (with reversed parameter order) not viable}}
+    bool operator==(int) const; // expected-note 2{{function not viable}}
   };
   struct E {
     bool operator==(const E&) const;
-    bool operator<(const E&) const = delete; // expected-note {{deleted}}
+    bool operator<(const E&) const = delete; // expected-note 2{{deleted}}
   };
   struct F {
-    std::strong_ordering operator<=>(const F&) const; // expected-note {{candidate}}
-    std::strong_ordering operator<=>(F) const; // expected-note {{candidate}}
+    std::strong_ordering operator<=>(const F&) const; // expected-note 2{{candidate}}
+    std::strong_ordering operator<=>(F) const; // expected-note 2{{candidate}}
   };
   struct G1 {
     bool operator==(const G1&) const;
@@ -108,6 +108,37 @@ namespace Deletedness {
       0
     );
   }
+
+  // expected-note@#arr {{deleted comparison function for member 'arr'}}
+  // expected-note@#arr {{no viable comparison function for member 'arr'}}
+  // expected-note@#arr {{three-way comparison cannot be synthesized because there is no viable function for '<' comparison}}
+  // expected-note@#arr {{no viable comparison function for member 'arr'}}
+  // expected-note@#arr {{three-way comparison cannot be synthesized because there is no viable function for '==' comparison}}
+  // expected-note@#arr {{deleted comparison function for member 'arr'}}
+  // expected-note@#arr {{implied comparison for member 'arr' is ambiguous}}
+  template<typename T> struct CmpArray {
+    T arr[3]; // #arr
+    std::strong_ordering operator<=>(const CmpArray&) const = default; // #cmparray expected-note 5{{here}}
+  };
+  void g() {
+    use(
+      CmpArray<A>() <=> CmpArray<A>(),
+      CmpArray<B>() <=> CmpArray<B>(),
+      CmpArray<C>() <=> CmpArray<C>(), // expected-error {{deleted}}
+      CmpArray<D1>() <=> CmpArray<D1>(), // expected-error {{deleted}}
+      CmpArray<D2>() <=> CmpArray<D2>(), // expected-error {{deleted}}
+      CmpArray<E>() <=> CmpArray<E>(), // expected-error {{deleted}}
+      CmpArray<F>() <=> CmpArray<F>(), // expected-error {{deleted}}
+      // FIXME: The following three errors are not very good.
+      // expected-error@#cmparray {{value of type 'void' is not contextually convertible to 'bool'}}
+      CmpArray<G1>() <=> CmpArray<G1>(), // expected-note-re {{in defaulted three-way comparison operator for '{{.*}}CmpArray<{{.*}}G1>' first required here}}j
+      // expected-error@#cmparray {{value of type 'void' is not contextually convertible to 'bool'}}
+      CmpArray<G2>() <=> CmpArray<G2>(), // expected-note-re {{in defaulted three-way comparison operator for '{{.*}}CmpArray<{{.*}}G2>' first required here}}j
+      // expected-error@#cmparray {{no matching conversion for static_cast from 'void' to 'std::strong_ordering'}}
+      CmpArray<H>() <=> CmpArray<H>(), // expected-note-re {{in defaulted three-way comparison operator for '{{.*}}CmpArray<{{.*}}H>' first required here}}j
+      0
+    );
+  }
 }
 
 namespace Synthesis {

diff  --git a/clang/test/CXX/class/class.compare/class.spaceship/p3.cpp b/clang/test/CXX/class/class.compare/class.spaceship/p3.cpp
index e4be892cd7c5..79fce259e4f9 100644
--- a/clang/test/CXX/class/class.compare/class.spaceship/p3.cpp
+++ b/clang/test/CXX/class/class.compare/class.spaceship/p3.cpp
@@ -10,14 +10,16 @@ namespace std {
 }
 
 struct A {
-  int a, b, c;
+  int a, b[3], c;
   std::strong_ordering operator<=>(const A&) const = default;
 };
 
-static_assert(A{1, 2, 3} <= A{1, 2, 3});
-static_assert(A{1, 2, 3} <= A{0, 20, 3}); // expected-error {{failed}}
-static_assert(A{1, 2, 3} <= A{1, 0, 30}); // expected-error {{failed}}
-static_assert(A{1, 2, 3} <= A{1, 2, 0}); // expected-error {{failed}}
+static_assert(A{1, 2, 3, 4, 5} <= A{1, 2, 3, 4, 5});
+static_assert(A{1, 2, 3, 4, 5} <= A{0, 20, 3, 4, 5}); // expected-error {{failed}}
+static_assert(A{1, 2, 3, 4, 5} <= A{1, 0, 30, 4, 5}); // expected-error {{failed}}
+static_assert(A{1, 2, 3, 4, 5} <= A{1, 2, 0, 40, 5}); // expected-error {{failed}}
+static_assert(A{1, 2, 3, 4, 5} <= A{1, 2, 3, 0, 50}); // expected-error {{failed}}
+static_assert(A{1, 2, 3, 4, 5} <= A{1, 2, 3, 4, 0}); // expected-error {{failed}}
 
 struct reverse_compare {
   int n;
@@ -26,10 +28,12 @@ struct reverse_compare {
 };
 
 struct B {
-  int a, b, c;
+  int a, b[3], c;
   friend reverse_compare operator<=>(const B&, const B&) = default;
 };
-static_assert(B{1, 2, 3} >= B{1, 2, 3});
-static_assert(B{1, 2, 3} >= B{0, 20, 3}); // expected-error {{failed}}
-static_assert(B{1, 2, 3} >= B{1, 0, 30}); // expected-error {{failed}}
-static_assert(B{1, 2, 3} >= B{1, 2, 0}); // expected-error {{failed}}
+static_assert(B{1, 2, 3, 4, 5} >= B{1, 2, 3, 4, 5});
+static_assert(B{1, 2, 3, 4, 5} >= B{0, 20, 3, 4, 5}); // expected-error {{failed}}
+static_assert(B{1, 2, 3, 4, 5} >= B{1, 0, 30, 4, 5}); // expected-error {{failed}}
+static_assert(B{1, 2, 3, 4, 5} >= B{1, 2, 0, 40, 5}); // expected-error {{failed}}
+static_assert(B{1, 2, 3, 4, 5} >= B{1, 2, 3, 0, 50}); // expected-error {{failed}}
+static_assert(B{1, 2, 3, 4, 5} >= B{1, 2, 3, 4, 0}); // expected-error {{failed}}


        


More information about the cfe-commits mailing list