[C++2a] Implement operator<=> Part 2: Operator Rewritting and Overload Resolution.

[EricWF]

Summary

This is a work-in-progress attempt to add operator<=> rewriting, including the required changes to overload resolution.

The rewritten expression is represented with the newly added AST node CXXRewrittenExpr. This node contains two sub-statements. The first is a "dummy" expression representing the syntactic form of the initial expression, where the input expressions are represented using OpaqueValueExpr. The second represents the rewritten expression, the input expressions are used here directly (ie are not wrapped in OpaqueValueExprs).

As currently implemented, rewritten and synthesized candidates are only partially checked for viability when they're added to the overload candidates (the conversion sequence from the argument type -> parameter type is checked, but nothing else). This can lead non-viable candidates being selected or causing ambiguity when final overload resolution is attempted.

The solution implemented in this patch is to fully build "potentially viable" candidates if they are selected or cause ambiguity. If building the candidate fails, it is marked as non-viable, and a new best viable function is computed.

The problem with this approach is that it's expensive and potentially wasteful. For example, if overload resolution results in ambiguity with N rewritten candidates with the same partial ordering, then all N candidates are fully built, and those results potentially thrown away.

For builtin candidates this can be avoided by separating out the bits of CheckBinaryOperands which compute it's result type from the bits which actually build the expression and convert the arguments. Once we know the return type of the builtin, we can deduce if the comparison category type can be used in the expression 0 @ <comp-category> without much effect.

However, for non-builtin overloads of operator<=> (which don't return a comparison category type), it seems non-trivial to check that 0 @ <result> is valid without actually attempting to build the overloaded binary operator.

[EricWF]

I'm guessing this implementation is super non-conforming, since expressions are evaluated during overload resolution.

My plan is to keep the general shape of this patch intact, but fix the eager building of expressions with something less wasteful.

[EricWF]

I know how to do better.

include/clang/Sema/Overload.h

938

This is unneeded. As long as the rewritten candidates are added last, restoring the Functions member is unneeded.

[rsmith]

lib/Sema/SemaOverload.cpp

839

I think this might be clearer if named getParamIndexForArgIndex or similar.

8880–8882

No \brief here (we use autobrief), and please use /// for continuation lines.

8888

Does the implementation below actually rely on this?

8901

No, you shouldn't be doing any lookups here. Instead, what you should do is to change the LookupOverloadedOperatorName call in BuildOverloadedBinOp to look up both operator@ and operator<=> in the case where @ is an equality or relational op, and then filter Fns by operator name both here and when adding the non-member function candidates.

The reason is that we need to do these lookups in the context of the template definition when an equality or relational expression appears within a template, so we need to look up both the original operator and <=> eagerly.

8936–8938

Type isn't sufficient to conclude this; you'll need to check the other relevant properties of LHS and RHS too (at least value kind, but maybe also bit-field-ness and others?).

9218–9219

You also need to check the reverse condition and return false (the "or if not that" is ... somehow ... supposed to imply that).

12502–12535

This results in an inadequate representation for the rewritten <=> form. We need to retain the "as-written" form of the operator, for multiple reasons (source fidelity for tooling, expression mangling, pretty-printing, ...). Generally, Clang's philosophy is to desugar the minimum amount that's necessary to capture both the syntactic and semantic forms.

A couple of possible representations come to mind here:

1. A specific Expr subclass for a rewritten <=> operator, which is just a wrapper around a (a <=> b) op 0 expression, and knows how to extract the subexpressions itself, or...
2. A general "rewritten operator wrapper" Expr subclass, which holds its operands and a rewritten expression, and for which the rewritten expression refers to its operands via OpaqueValueExprs. We already have such a class (PseudoObjectExpr).

The former is a smaller and simpler representation, but will likely be more work to implement.

12537–12565

Hmm, this is an interesting approach. It's quite a divergence from our usual approach, which is to perform the viability checks first, before partial ordering, even if we could eliminate some candidates based on partial ordering. And it's observable -- it will result in fewer template instantiations being performed, which will mean we do not diagnose some ill-formed (no diagnostic required) situations which would otherwise fail due to an error in a non-immediate context.

Can you say something about why you chose this approach instead of doing the lookup for operator @ early and marking the candidate non-viable if the lookup / overload resolution for it fails?

(I'm also vaguely wondering whether we should be pushing back on the C++ committee for making viability of the rewritten candidate depend on validity of the @ expression, not just the <=> expression.)

12570–12572

This is a really interesting situation. What does it even mean to consider the (a <=> b) @ c expression if lookup for <=> finds a deleted function? (It would be novel for the return type of a deleted function to be meaningful, and it would be novel to exclude an overload candidate because it's deleted.) And what if lookup for @ finds a deleted function?

I think this is good reason to go back to CWG and suggest that we don't check the @ expression at all until after overload resolution.

12608–12609

This is not correct, because it turns out that isBetterOverloadCandidate is not a strict weak order. It's not any kind of order at all, actually, since it lacks the transitivity property. I don't know whether you can do better than marking your chosen candidate non-viable and rerunning the candidate selection algorithm if your presumed-best candidate turns out to be non-viable.

12634

I am a vampire and

[EricWF]

lib/Sema/SemaOverload.cpp

8888

Like should it be possible to hit this line of code? No. It was preemptive. I'll remove it.

12537–12565

The main reason for choosing this approach was the current expense of validating the candidates eagerly when they're added. Currently builtin candidates need to be fully built before their viability is known. My initial attempt at this made the compiler just about hang.

If I understand what you mean by performing lookup/overload resolution early for operator @, and I probably don't, then it would require performing overload resolution separately every rewritten candidate added, since each candidate could have a different return type.

I think rewritten builtins could be checked more eagerly by moving a bunch of the checking from SemaExpr.cpp to SemaOverload.cpp, and using it to eagerly check the bits we need to, but not actually building the expression as calling into SemaExpr.cpp would currently do.

As for the root question: Should the validity of the @ expression be considered during overload resolution? I think the answer is tricky. For example, the following case could easily occur and break existing code, especially as more users add defaulted comparison operators. In this case if the base class adds <=> without the derived classes knowledge.

struct B {
  using CallbackTy = void(*)();
  CallbackTy CB = nullptr; // B carries around a function pointer.
#if __cplusplus >= 201703L
  friend auto operator<=>(B const&, B const&) = default;
#else
  friend bool operator==(B, B);
  friend bool operator!=(B, B);
#endif
};
struct D {
  operator int() const; // D uses this conversion to provide comparisons.
};
D d;
// comparison ill-formed if the `@` expression isn't considered for viability.
auto r = d < d;

However, allowing the @ expression to effect viability leads to a different set of bugs. For example this slicing comparison bug:

struct B {
  int x;
  friend auto operator<=>(B, B) = default;
};
struct D : B {
  decltype(nullptr) y = {};
  friend auto operator<=>(D, D) = default;
};
D d;
// If the `@` expression is considered for viability, then `operator<=>(B, B)` will be used, effectively slicing the data used during the comparison. 
auto r = d < d;

What is clear is that it should go back to CWG for more discussion.

12570–12572

What does it even mean to consider the (a <=> b) @ c expression if lookup for <=> finds a deleted function?

Separate from allowing return types to affect overload resolution, I suspect selecting a deleted rewritten candidate means that we should have never considered the candidate in the first place. For example:

struct B { auto operator<=>(B) const = delete; };
struct D : private B { operator int() const; };
D{} < D{};

In the above case, the intent of the code seems clear. But because the compiler invented a <=> candidate with a better conversion ranking, our code is ill-formed. Therefore, it makes sense to prefer non-rewritten candidates over deleted rewritten ones.

That said, it certainly doesn't make sense to "make way" for a worse rewritten candidate when the best viable function is deleted. For example:

struct Any { Any(...); };
auto operator<=>(Any, Any);
struct T { friend auto operator<=>(T, T) = delete; };
T{} < T{}; // selecting operator<=>(Any, Any) would be bad.

12608–12609

I was afraid of that, but I bit too tired to initially check this. Thanks for the correction.

If I'm not mistaken, isBetterOverloadCandidate should still be able to provide list of equivalent candidates, which could be used when handling this case.

12634

Ha. I had to look that reference up.

[EricWF]

lib/Sema/SemaOverload.cpp

12537–12565

It's quite a divergence from our usual approach, which is to perform the viability checks first, before partial ordering, even if we could eliminate some candidates based on partial ordering. And it's observable -- it will result in fewer template instantiations being performed, which will mean we do not diagnose some ill-formed (no diagnostic required) situations which would otherwise fail due to an error in a non-immediate context.

I should clarify that the same validity checks are applied to rewritten candidates are also applied to non-rewritten candidates, such as checking conversions to parameter types. So I suspect, for all intents and purposes, it should produce the same amount of template instantiations as existing operator lookup does.

[EricWF]

lib/Sema/SemaOverload.cpp

9218–9219

Hmm. So I'm wondering what is intended by the language F1 and F2 are rewritten candidates, and F2 is a synthesized candidate with reversed order of parameters and F1 is not. For example, what happens when comparing two distinct member functions with only one explicit parameter?

struct T;
struct U { auto operator<=>(T); };
struct T { auto operator<=>(U); };
auto r = T{} < U{}; // Are the synthesized and rewritten overloads ambiguous?

[EricWF]

lib/Sema/SemaOverload.cpp

9218–9219

And what about

struct U {};
struct T { auto operator<=>(U); };
auto operator<=>(U, T);
auto r = T{} < U{};

[EricWF]

lib/Sema/SemaOverload.cpp

9218–9219

Nevermind. I found the language. The implicit object parameter isn't considered in this case.

[rsmith]

Definitely some interesting questions to take to CWG here. :)

lib/Sema/SemaOverload.cpp

9218–9219

The parameters in scope in this rule are the parameters for the purpose of overload resolution, which include the implicit object parameter for a member function.

In your first example, both candidates have implicit conversion sequences with Exact Match rank for both parameters (and none of the ordering special cases apply), so we fall down to this tie-breaker and it selects the T::operator<=>(U) candidate, unless I'm missing something.

Your second example appears to receive the same treatment.

12537–12565

I'm not sure that it's true in general that we get the same amount of instantiation with this approach. For example, checking the @ expression will trigger instantiation of the return type of the operator<=>, and may trigger instantiation of the operator<=> function itself in some cases. (I think this is a conforming technique; I think the more interesting question is whether it's a good thing. I believe GCC uses techniques like this, and it does create portability problems in practice for code written against GCC, for example -- but on the other hand, they presumably spend less time in overload resolution as a result.)

12570–12572

I'm not sure I see clear intent in your first example. B is saying "any attempt to compare me is ill-formed". It's not clear to me what D's intent when privately inheriting from B is. But if the operator<=> were not deleted, we would[1] have selected it (and such selection would even be valid if done in a context in which B is an accessible base class of D) -- so I think that program should be rejected.

[1]: Well, here's the problem. We would have selected it if it returned a type that supports < 0. And that's not a meaningful question to ask. I suppose we could argue that if the "= delete" were simply removed, we'd have rejected it due to trying to use a function with a deduced return type before the return type is available.

[EricWF]

lib/Sema/SemaOverload.cpp

9218–9219

Alright. So I found the wrong language.

I think my first case meant to have const & parameters and const methods. That way the implicit object parameter type matched the reversed parameter type. The second example was meant to demonstrate the case where the implicit object parameter type doesn't match.

What type should we be comparing against in these cases?

[EricWF]

Updating this with the latest work. This adds the semantics for checking and generating default comparison operators.

The overload resolution bits are still a mess, and I'll clean those up shortly. Also there are not sufficient tests yet, those will also come shortly.
I also need to go through the wording for defaulted functions again, to ensure the constexpr and noexcept deduction is correct, as well
as the diagnostics for deleted comparisons.

One oddity introduced here is that defaulted comparisons are evaluated using the "Special Member" matchinary, even though they're
not technically special members, or even members. However, it seemed like the appropriate place to implement them. I'll probably rename
most FooSpecialMember functions to FooDefaultedMember to clear up any confusion.

[EricWF]

• Better integrate defaulted comparison with other special members; this buys correct access, constexpr, and except specification checking.

However it's not clear if this is the best approach. Lookups for defaulted comparison can't be cached like other special members, because the lookup can change after the class definition. It is clear, however, that using the existing machinery would be too expensive if not for cached lookup results, because the checks for constexpr, deletion, and noexcept all depend on the results for lookup being cached.

[rsmith]

Can you split this patch up a bit? There's changes here for the <=> rewriting, but also for supporting operator<=>(...) = default, and some constant evaluation and code generation changes too. It's a lot easier to review more directed, smaller patches.

include/clang/AST/ExprCXX.h

4212 ↗

(On Diff #143433)

This should have a name ending Expr.

4230–4231 ↗

(On Diff #143433)

It probably won't ever be type-dependent, but could absolutely be value-dependent, instantiation-dependent, or contain an unexpanded parameter pack.

4294–4296 ↗

(On Diff #143433)

I find the interface exposed by this class a little confusing. On the one hand, it provides the original expression as an Expr*, and on the other hand it hard-codes knowledge about the form of that expression and exposes accessors into that original form. The upshot is that we pay the cost of a general mechanism (that can cope with any form of original expression) but don't get any benefit from that (because the class hard-codes non-generality).

This also generates a distinctly non-tree-shaped AST; clients naively walking recursively over expressions and their subexpressions will see the same`Expr*` multiple times and potentially end up performing an exponential-time tree walk.

I think there are (at least) two reasonable approaches here:

1. Use OpaqueValueExprs to handle the repeated AST nodes, and track an original expression, a rewritten expression, and probably an original LHS expr and an original RHS expr; the original and rewrite would use OVEs to refer indirectly to the LHS and RHS. Remove all hardcoded knowledge of the original and rewrite from this expression node and make it generic for rewritten expression forms.

2. Make this node explicitly be specific to spaceship rewrites. Remove the original expression pointer and rename it to something more specific. Keep the accessors that make hardcoded assumptions about the form of the rewritten expression.

Option 2 looks closer to what this patch is currently doing (you're not using the original expression much).

lib/AST/ItaniumMangle.cpp

3555 ↗

(On Diff #143433)

No, the expression should mangle as-written.

[EricWF]

• Remove the = default changes as requested.

• Attempt to work around ambiguity in overload resolution caused by differing lvalue or qualification conversions when ranking synthesized candidates. For example:

struct T {};
auto operator<=>(T const&, T&&);
T{} <=> T{}; // Ambiguous according to the current spec.

The workaround ignores worse implicit conversions when the conversions are both an exact match and a rewritten candidate tie-breaker is present.

[EricWF]

lib/Sema/SemaOverload.cpp

9177

This doesn't seem anywhere close to correct, but It's the best I've come up with so far.
For now though, it seems nice to avoid the ambiguity caused by the current specification.

[EricWF]

Generalize CXXRewrittenExpr to contain only a "representation" of the original expression, and the fully checked rewritten expression. By "representation" i mean to imply that we can't actually build and check a full representation for the expression as written, because it need not be valid. For example:

struct T { auto operator<=>(T const&) const = default; };
T{} < T{};

Currently this patch builds a BinaryOperator node to represent the expression as written, regardless of whether the original expression would actually select a builtin comparison operator. A consequence of this pseudo-representation is that we can't effectively transform it in TreeTransform.h (currently we only transform the rewritten expression and not the representation of the original; there are some kinks I need to work out there).

[EricWF]

• Further generalize CXXRewrittenExpr and get it working with TreeTransform.h.

[EricWF]

• Rebase on master.

I think the correct direction to head with this patch is to start removing the bits of the implementation which evaluate and build rewritten expressions during overload resolution. I'll submit such an update tomorrow.

[EricWF]

I'm still not sure how to best generate a diff that best ignores white spaces changes caused by clang-format or my editor.

I'll work on removing existing whitespace changes.

[EricWF]

Remove a bunch of nonsense this patch originally had. Specifically remove the bits of overload resolution which disambiguated rewritten expressions after the fact; Instead attempt to validate that the return type of the three-way comparison operand in a rewritten relational or equality can be used with the the specified operand (e.g. std::strong_equality cannot be used with a relational operator).

[rsmith]

include/clang/AST/ExprCXX.h

4294–4296 ↗

(On Diff #143433)

As noted, if you want this to be a general-purpose "rewritten expression" node, you should use OpaqueValueExprs to track the repeated AST nodes between the original and rewritten expression forms rather than forming a non-tree-shaped AST. However, if you want to take that path, you don't need to add a new Expr node for that; that's what PseudoObjectExpr is for.

4218–4226 ↗

(On Diff #146090)

I don't think you need this.

include/clang/AST/Stmt.h

254 ↗

(On Diff #146090)

I don't think you need this either.

include/clang/Sema/Overload.h

81–83

These names are not very good. The standard describes both these cases as "rewritten", so ROC_Rewritten is ambiguous, and it says "a synthesized candidate with reversed order of parameters", not merely "synthesized" (which would fail to communicate what's going on).

How about ROC_AsThreeWayComparison and ROC_AsReversedThreeWayComparison or similar?

938

Please instead extend the Functions set to be a set of PointerIntPair<Decl*, 2, RewrittenOperatorCandidateKind>.

include/clang/Serialization/ASTBitCodes.h

1814 ↗

(On Diff #146090)

Drop the _OPERATOR here.

lib/Sema/SemaOverload.cpp

12542

in a *call* to a

12543–12547

If you want to model this as a generic (syntactic form, semantic form) pair, the syntactic form needs to preserve enough information that TreeTransform on it can recreate the semantic form. That means you need to store a CXXOperatorCallExpr to an UnresolvedLookupExpr to hold Fns if Fns is not empty (see the code at the start of CreateOverloadedBinOp that deals with the type-dependent case for an example of what you would need to do).

Though perhaps this is as good a reason as any to give up the idea of a generic rewrite expression and add something specific to a three-way comparison rewrite; the representation needed to fit this into a generic rewrite mechanism is very unwieldy. (If we add any more forms of rewritten operator later, we can consider whether a generalization is worthwhile.)

lib/Sema/TreeTransform.h

11569–11573 ↗

(On Diff #146090)

This doesn't seem right to me; TreeTransform should be transforming the original expression, not the rewritten one. We shouldn't be assuming we know what kind of transforms the client wants to perform, it's just our job to give them the syntactic pieces and ask for those pieces to be transformed.

(But this is all redundant if you switch to using PseudoObjectExpr.)

[EricWF]

• Change CXXRewrittenExpr to be a non-general wrapper only for rewritten comparison operators.
• Start fixing and improving overload resolution diagnostics.

[EricWF]

include/clang/AST/ExprCXX.h

4218–4226 ↗

(On Diff #146090)

I guess it's just future proofing, or an attempt to demonstrate how the class may be generalized. I'll remove it for now.

include/clang/Sema/Overload.h

81–83

Sounds good to me.

938

Sure. I'll get that done.

There are two options here:

(A) Keep the guard object. Store the RewrittenOverloadCandidateKind which we're currently adding candidates for, and have addCandidate and isNewCandidate handle the ROC.

(B) Change all of the AddFooCandidates interfaces in Sema to take yet another default parameter, and pass it around everywhere.

Which would you prefer @rsmith?

lib/Sema/SemaOverload.cpp

12543–12547

I think the FIXME comment may be incorrect.

Shouldn't this be sufficient since we never actually build the rewritten expression until the input expressions are no longer type dependent?
Additionally, with this representation TreeTransform can transform the semantic form, since (if I'm correct), a rebuild should never re-perform overload resolution to choose how to rewrite the initial expression. (Does that make sense, and is it correct?)

I'll go ahead and start converting to a less-general representation of the rewritten expression.

[EricWF]

lib/Sema/SemaOverload.cpp

12543–12547

Nevermind. This was answered offline.

[EricWF]

include/clang/AST/Stmt.h

254 ↗

(On Diff #146090)

Does keeping it help save bits in CXXRewrittenOperatorExpr?

[EricWF]

Do better english in diagnostics.

[EricWF]

Fix fetching the correct source location information in CXXRewrittenOperatorExpr.

[EricWF]

• Update tests and improve diagnostics. Still more work to be done here though.

include/clang/Sema/Overload.h

enum OverloadCandidateDisplayKind {
     /// Requests that all candidates be shown.  Viable candidates will
     /// be printed first.
     OCD_AllCandidates,
 
     /// Requests that only viable candidates be shown.
     OCD_ViableCandidates
   };
 
+  /// OperatorOverloadCandidateKind - The kind of the operator candidate in
+  /// accordance with [over.match.oper].
+  enum RewrittenOverloadCandidateKind : unsigned char {
+    /// Not a rewritten candidate.
+    ROC_None,
+    /// Rewritten but not synthesized.
+    ROC_Rewritten,
+    /// Both rewritten and synthesized.
+    ROC_Synthesized
+  };
+
   /// ImplicitConversionKind - The kind of implicit conversion used to
   /// convert an argument to a parameter's type. The enumerator values
   /// match with the table titled 'Conversions' in [over.ics.scs] and are listed
   /// such that better conversion kinds have smaller values.
   enum ImplicitConversionKind {
     /// Identity conversion (no conversion)
     ICK_Identity = 0,
 
struct OverloadCandidate {
     /// The conversion sequences used to convert the function arguments
     /// to the function parameters.
     ConversionSequenceList Conversions;
 
     /// The FixIt hints which can be used to fix the Bad candidate.
     ConversionFixItGenerator Fix;
 
     /// Viable - True to indicate that this overload candidate is viable.
-    bool Viable;
+    bool Viable : 1;
 
     /// IsSurrogate - True to indicate that this candidate is a
     /// surrogate for a conversion to a function pointer or reference
     /// (C++ [over.call.object]).
-    bool IsSurrogate;
+    bool IsSurrogate : 1;
 
     /// IgnoreObjectArgument - True to indicate that the first
     /// argument's conversion, which for this function represents the
     /// implicit object argument, should be ignored. This will be true
     /// when the candidate is a static member function (where the
     /// implicit object argument is just a placeholder) or a
     /// non-static member function when the call doesn't have an
     /// object argument.
-    bool IgnoreObjectArgument;
+    bool IgnoreObjectArgument : 1;
+
+    /// RewrittenKind - For rewritten operator candidates, the kind of rewritten
+    /// candidate it is: rewritten or synthesized.
+    unsigned char RewrittenOpKind : 2;
 
     /// FailureKind - The reason why this candidate is not viable.
     /// Actually an OverloadFailureKind.
     unsigned char FailureKind;
 
     /// \brief The number of call arguments that were explicitly provided,
     /// to be used while performing partial ordering of function templates.
     unsigned ExplicitCallArguments;
bool TryToFixBadConversion(unsigned Idx, Sema &S) {
 
       // If at least one conversion fails, the candidate cannot be fixed.
       if (!CanFix)
         Fix.clear();
 
       return CanFix;
     }
 
+    /// \brief Return the "true" index for the specified argument. If this
+    /// is not a synthesized candidate, 'Idx' is returned. Otherwise the
+    /// index corresponding to the reversed parameter is returned.
+    unsigned getTrueArgIndex(unsigned Idx) const;
+
+    /// \brief Return the conversion sequence for the specified argument index.
+    /// If this is a synthesized candidate, the argument index is reversed.
+    const ImplicitConversionSequence &getConversion(unsigned ArgIdx) const;
+
+    /// \brief Return the parameter type for the specified index. If this is
+    /// a synthesized candidate, the argument index is reversed.
+    QualType getParamType(unsigned ArgIdx) const;
+
     unsigned getNumParams() const {
       if (IsSurrogate) {
         auto STy = Surrogate->getConversionType();
         while (STy->isPointerType() || STy->isReferenceType())
           STy = STy->getPointeeType();
         return STy->getAs<FunctionProtoType>()->getNumParams();
       }
       if (Function)
         return Function->getNumParams();
       return ExplicitCallArguments;
     }
+
+    RewrittenOverloadCandidateKind getRewrittenKind() const {
+      return static_cast<RewrittenOverloadCandidateKind>(RewrittenOpKind);
+    }
   };
 
   /// OverloadCandidateSet - A set of overload candidates, used in C++
   /// overload resolution (C++ 13.3).
   class OverloadCandidateSet {
   public:
     enum CandidateSetKind {
       /// Normal lookup.
enum CandidateSetKind {
       /// C++ [over.match.ctor], [over.match.list]
       /// Initialization of an object of class type by constructor,
       /// using either a parenthesized or braced list of arguments.
       CSK_InitByConstructor,
     };
 
   private:
     SmallVector<OverloadCandidate, 16> Candidates;
-    llvm::SmallPtrSet<Decl *, 16> Functions;
+    using DeclSet = llvm::SmallPtrSet<Decl *, 16>;
+    DeclSet Functions;
 
+  private:
     // Allocator for ConversionSequenceLists. We store the first few of these
     // inline to avoid allocation for small sets.
     llvm::BumpPtrAllocator SlabAllocator;
 
     SourceLocation Loc;
     CandidateSetKind Kind;
 
     constexpr static unsigned NumInlineBytes =
T *slabAllocate(unsigned N) {
 
       NumInlineBytesUsed += NBytes;
       return reinterpret_cast<T *>(FreeSpaceStart);
     }
 
     void destroyCandidates();
 
   public:
+    /// \brief RewrittenCandidateContextGuard - Enter a context suitable for
+    /// adding rewritten overload candidates. Rewritten candidates can
+    /// re-consider previously seen functions, so save and clear the list of
+    /// considered functions, and restore it when the rewritten context is
+    /// exited.
+    struct RewrittenCandidateContextGuard {
+      RewrittenCandidateContextGuard(OverloadCandidateSet &CS)
+          : CandidateSet(CS) {
+        assert(CS.Kind == CSK_Operator &&
+               "rewritten expressions can only occur for operators");
+        OldFunctions = std::move(CandidateSet.Functions);
+      }
+
+      ~RewrittenCandidateContextGuard() {
+        CandidateSet.Functions.insert(OldFunctions.begin(), OldFunctions.end());
+      }
+
+    private:
+      OverloadCandidateSet &CandidateSet;
+      DeclSet OldFunctions;
+    };
+
+    friend struct RewrittenCandidateContextGuard;
+
+  public:
     OverloadCandidateSet(SourceLocation Loc, CandidateSetKind CSK)
         : Loc(Loc), Kind(CSK) {}
     OverloadCandidateSet(const OverloadCandidateSet &) = delete;
     OverloadCandidateSet &operator=(const OverloadCandidateSet &) = delete;
     ~OverloadCandidateSet() { destroyCandidates(); }
 
     SourceLocation getLocation() const { return Loc; }
     CandidateSetKind getKind() const { return Kind; }

include/clang/Sema/Sema.h

                                    SourceRange OpRange = SourceRange());
   void AddBuiltinCandidate(QualType *ParamTys, ArrayRef<Expr *> Args,
                            OverloadCandidateSet& CandidateSet,
                            bool IsAssignmentOperator = false,
                            unsigned NumContextualBoolArguments = 0);
   void AddBuiltinOperatorCandidates(OverloadedOperatorKind Op,
                                     SourceLocation OpLoc, ArrayRef<Expr *> Args,
                                     OverloadCandidateSet& CandidateSet);
+  void AddRewrittenOperatorCandidates(OverloadedOperatorKind Op,
+                                      SourceLocation OpLoc,
+                                      ArrayRef<Expr *> Args,
+                                      OverloadCandidateSet &CandidateSet,
+                                      bool PerformADL);
   void AddArgumentDependentLookupCandidates(DeclarationName Name,
                                             SourceLocation Loc,
                                             ArrayRef<Expr *> Args,
                                 TemplateArgumentListInfo *ExplicitTemplateArgs,
                                             OverloadCandidateSet& CandidateSet,
                                             bool PartialOverloading = false);
 
   // Emit as a 'note' the specific overload candidate

                               OverloadCandidateSet *CandidateSet,
                               ExprResult *Result);
 
   ExprResult CreateOverloadedUnaryOp(SourceLocation OpLoc,
                                      UnaryOperatorKind Opc,
                                      const UnresolvedSetImpl &Fns,
                                      Expr *input, bool RequiresADL = true);
 
-  ExprResult CreateOverloadedBinOp(SourceLocation OpLoc,
-                                   BinaryOperatorKind Opc,
-                                   const UnresolvedSetImpl &Fns,
-                                   Expr *LHS, Expr *RHS,
-                                   bool RequiresADL = true);
+  ExprResult CreateOverloadedBinOp(SourceLocation OpLoc, BinaryOperatorKind Opc,
+                                   const UnresolvedSetImpl &Fns, Expr *LHS,
+                                   Expr *RHS, bool RequiresADL = true,
+                                   bool AllowRewrittenCandidates = true);
 
   ExprResult CreateOverloadedArraySubscriptExpr(SourceLocation LLoc,
                                                 SourceLocation RLoc,
                                                 Expr *Base,Expr *Idx);
 
   ExprResult
   BuildCallToMemberFunction(Scope *S, Expr *MemExpr,
                             SourceLocation LParenLoc,

   bool CheckPointerCall(NamedDecl *NDecl, CallExpr *TheCall,
                         const FunctionProtoType *Proto);
   bool CheckOtherCall(CallExpr *TheCall, const FunctionProtoType *Proto);
   void CheckConstructorCall(FunctionDecl *FDecl,
                             ArrayRef<const Expr *> Args,
                             const FunctionProtoType *Proto,
                             SourceLocation Loc);
 
+public:
   void checkCall(NamedDecl *FDecl, const FunctionProtoType *Proto,
                  const Expr *ThisArg, ArrayRef<const Expr *> Args,
                  bool IsMemberFunction, SourceLocation Loc, SourceRange Range,
                  VariadicCallType CallType);
 
+private:
   bool CheckObjCString(Expr *Arg);
   ExprResult CheckOSLogFormatStringArg(Expr *Arg);
 
   ExprResult CheckBuiltinFunctionCall(FunctionDecl *FDecl,
                                       unsigned BuiltinID, CallExpr *TheCall);
 
   bool CheckARMBuiltinExclusiveCall(unsigned BuiltinID, CallExpr *TheCall,
                                     unsigned MaxWidth);

lib/Sema/SemaOverload.cpp

   case Sema::TDK_DeducedMismatchNested:
     return static_cast<DFIDeducedMismatchArgs*>(Data)->CallArgIndex;
 
   default:
     return llvm::None;
   }
 }
 
+const ImplicitConversionSequence &
+OverloadCandidate::getConversion(unsigned ArgIdx) const {
+  return Conversions[getTrueArgIndex(ArgIdx)];
+}
+
+unsigned OverloadCandidate::getTrueArgIndex(unsigned Idx) const {
+  if (getRewrittenKind() != ROC_Synthesized)
+    return Idx;
+  // FIXME(EricWF): Handle these cases.
+  assert(!IsSurrogate);
+  assert(!IgnoreObjectArgument);
+  // assert(getNumParams() == 2);
+  assert(Idx < 2);
+  return Idx == 0 ? 1 : 0;
+}
+
+QualType OverloadCandidate::getParamType(unsigned ArgIdx) const {
+  ArgIdx = getTrueArgIndex(ArgIdx);
+  if (Function)
+    return Function->getParamDecl(ArgIdx)->getType();
+  return BuiltinParamTypes[ArgIdx];
+}
+
 void OverloadCandidateSet::destroyCandidates() {
   for (iterator i = begin(), e = end(); i != e; ++i) {
     for (auto &C : i->Conversions)
       C.~ImplicitConversionSequence();
     if (!i->Viable && i->FailureKind == ovl_fail_bad_deduction)
       i->DeductionFailure.Destroy();
   }
 }

   // C++ [over.match.oper]p3:
   //   if no operand has a class type, only those non-member functions in the
   //   lookup set that have a first parameter of type T1 or "reference to
   //   (possibly cv-qualified) T1", when T1 is an enumeration type, or (if there
   //   is a right operand) a second parameter of type T2 or "reference to
   //   (possibly cv-qualified) T2", when T2 is an enumeration type, are
   //   candidate functions.
   if (CandidateSet.getKind() == OverloadCandidateSet::CSK_Operator &&
-      !IsAcceptableNonMemberOperatorCandidate(Context, Function, Args))
+      !IsAcceptableNonMemberOperatorCandidate(Context, Function, Args)) {
     return;
+  }
 
   // C++11 [class.copy]p11: [DR1402]
   //   A defaulted move constructor that is defined as deleted is ignored by
   //   overload resolution.
   CXXConstructorDecl *Constructor = dyn_cast<CXXConstructorDecl>(Function);
+
   if (Constructor && Constructor->isDefaulted() && Constructor->isDeleted() &&
       Constructor->isMoveConstructor())
     return;
 
   // Overload resolution is always an unevaluated context.
   EnterExpressionEvaluationContext Unevaluated(
       *this, Sema::ExpressionEvaluationContext::Unevaluated);
 
   // Add this candidate
   OverloadCandidate &Candidate =
       CandidateSet.addCandidate(Args.size(), EarlyConversions);
   Candidate.FoundDecl = FoundDecl;
   Candidate.Function = Function;
   Candidate.Viable = true;
   Candidate.IsSurrogate = false;
+  Candidate.RewrittenOpKind = ROC_None;
   Candidate.IgnoreObjectArgument = false;
   Candidate.ExplicitCallArguments = Args.size();
 
   if (Function->isMultiVersion() &&
       !Function->getAttr<TargetAttr>()->isDefaultVersion()) {
     Candidate.Viable = false;
     Candidate.FailureKind = ovl_non_default_multiversion_function;
     return;

                 ObjectClassification);
           })) {
     OverloadCandidate &Candidate =
         CandidateSet.addCandidate(Conversions.size(), Conversions);
     Candidate.FoundDecl = FoundDecl;
     Candidate.Function = MethodTmpl->getTemplatedDecl();
     Candidate.Viable = false;
     Candidate.IsSurrogate = false;
+    Candidate.RewrittenOpKind = ROC_None;
     Candidate.IgnoreObjectArgument =
         cast<CXXMethodDecl>(Candidate.Function)->isStatic() ||
         ObjectType.isNull();
     Candidate.ExplicitCallArguments = Args.size();
     if (Result == TDK_NonDependentConversionFailure)
       Candidate.FailureKind = ovl_fail_bad_conversion;
     else {
       Candidate.FailureKind = ovl_fail_bad_deduction;

                                                 SuppressUserConversions);
           })) {
     OverloadCandidate &Candidate =
         CandidateSet.addCandidate(Conversions.size(), Conversions);
     Candidate.FoundDecl = FoundDecl;
     Candidate.Function = FunctionTemplate->getTemplatedDecl();
     Candidate.Viable = false;
     Candidate.IsSurrogate = false;
+    Candidate.RewrittenOpKind = ROC_None;
     // Ignore the object argument if there is one, since we don't have an object
     // type.
     Candidate.IgnoreObjectArgument =
         isa<CXXMethodDecl>(Candidate.Function) &&
         !isa<CXXConstructorDecl>(Candidate.Function);
     Candidate.ExplicitCallArguments = Args.size();
     if (Result == TDK_NonDependentConversionFailure)
       Candidate.FailureKind = ovl_fail_bad_conversion;

   EnterExpressionEvaluationContext Unevaluated(
       *this, Sema::ExpressionEvaluationContext::Unevaluated);
 
   // Add this candidate
   OverloadCandidate &Candidate = CandidateSet.addCandidate(1);
   Candidate.FoundDecl = FoundDecl;
   Candidate.Function = Conversion;
   Candidate.IsSurrogate = false;
+  Candidate.RewrittenOpKind = ROC_None;
   Candidate.IgnoreObjectArgument = false;
   Candidate.FinalConversion.setAsIdentityConversion();
   Candidate.FinalConversion.setFromType(ConvType);
   Candidate.FinalConversion.setAllToTypes(ToType);
   Candidate.Viable = true;
   Candidate.ExplicitCallArguments = 1;
 
   // C++ [over.match.funcs]p4:

         = DeduceTemplateArguments(FunctionTemplate, ToType,
                                   Specialization, Info)) {
     OverloadCandidate &Candidate = CandidateSet.addCandidate();
     Candidate.FoundDecl = FoundDecl;
     Candidate.Function = FunctionTemplate->getTemplatedDecl();
     Candidate.Viable = false;
     Candidate.FailureKind = ovl_fail_bad_deduction;
     Candidate.IsSurrogate = false;
+    Candidate.RewrittenOpKind = ROC_None;
     Candidate.IgnoreObjectArgument = false;
     Candidate.ExplicitCallArguments = 1;
     Candidate.DeductionFailure = MakeDeductionFailureInfo(Context, Result,
                                                           Info);
     return;
   }
 
   // Add the conversion function template specialization produced by

       *this, Sema::ExpressionEvaluationContext::Unevaluated);
 
   OverloadCandidate &Candidate = CandidateSet.addCandidate(Args.size() + 1);
   Candidate.FoundDecl = FoundDecl;
   Candidate.Function = nullptr;
   Candidate.Surrogate = Conversion;
   Candidate.Viable = true;
   Candidate.IsSurrogate = true;
+  Candidate.RewrittenOpKind = ROC_None;
   Candidate.IgnoreObjectArgument = false;
   Candidate.ExplicitCallArguments = Args.size();
 
   // Determine the implicit conversion sequence for the implicit
   // object parameter.
   ImplicitConversionSequence ObjectInit = TryObjectArgumentInitialization(
       *this, CandidateSet.getLocation(), Object->getType(),
       Object->Classify(Context), Conversion, ActingContext);

   EnterExpressionEvaluationContext Unevaluated(
       *this, Sema::ExpressionEvaluationContext::Unevaluated);
 
   // Add this candidate
   OverloadCandidate &Candidate = CandidateSet.addCandidate(Args.size());
   Candidate.FoundDecl = DeclAccessPair::make(nullptr, AS_none);
   Candidate.Function = nullptr;
   Candidate.IsSurrogate = false;
+  Candidate.RewrittenOpKind = ROC_None;
   Candidate.IgnoreObjectArgument = false;
   std::copy(ParamTys, ParamTys + Args.size(), Candidate.BuiltinParamTypes);
 
   // Determine the implicit conversion sequences for each of the
   // arguments.
   Candidate.Viable = true;
   Candidate.ExplicitCallArguments = Args.size();
   for (unsigned ArgIdx = 0, N = Args.size(); ArgIdx != N; ++ArgIdx) {

 
   case OO_Conditional:
     OpBuilder.addConditionalOperatorOverloads();
     OpBuilder.addGenericBinaryArithmeticOverloads();
     break;
   }
 }
 
+/// \brief Add the rewritten and synthesized candidates for binary comparison
+//    operators. No additional semantic checking is done to see if the candidate
+//    is well formed.
+void Sema::AddRewrittenOperatorCandidates(OverloadedOperatorKind Op,
+                                          SourceLocation OpLoc,
+                                          ArrayRef<Expr *> InputArgs,
+                                          OverloadCandidateSet &CandidateSet,
+                                          bool PerformADL) {
+  assert(getLangOpts().CPlusPlus2a);
+  auto Opc = BinaryOperator::getOverloadedOpcode(Op);
+
+  bool IsRelationalOrEquality =
+      BinaryOperator::isRelationalOp(Opc) || BinaryOperator::isEqualityOp(Opc);
+  if (!IsRelationalOrEquality && Opc != BO_Cmp)
+    return;
+  assert(InputArgs.size() == 2);
+
+  OverloadedOperatorKind CmpOp = OO_Spaceship;
+  DeclarationName OpName = Context.DeclarationNames.getCXXOperatorName(CmpOp);
+
+  // Lookup possible candidates for the rewritten operator.
+  // FIXME:  should this really be done in the current scope?
+  LookupResult Operators(*this, OpName, SourceLocation(),
+                         Sema::LookupOperatorName);
+  LookupName(Operators, getCurScope());
+  assert(!Operators.isAmbiguous() && "Operator lookup cannot be ambiguous");
+  const auto &Functions = Operators.asUnresolvedSet();
+
+  // AddCandidates - Add operator<=> candidates for the specified set of args,
+  // and mark all newly generated candidates as having the specified
+  // 'RewrittenOverloadCandidateKind'.
+  auto AddCandidates = [&](ArrayRef<Expr *> Args,
+                           RewrittenOverloadCandidateKind Kind) {
+    OverloadCandidateSet::RewrittenCandidateContextGuard Guard(CandidateSet);
+
+    unsigned InitialSize = CandidateSet.size();
+    AddFunctionCandidates(Functions, Args, CandidateSet);
+    AddMemberOperatorCandidates(CmpOp, OpLoc, Args, CandidateSet);
+    if (PerformADL)
+      AddArgumentDependentLookupCandidates(OpName, OpLoc, Args,
+                                           /*ExplicitTemplateArgs*/ nullptr,
+                                           CandidateSet);
+    AddBuiltinOperatorCandidates(CmpOp, OpLoc, Args, CandidateSet);
+
+    for (auto It = std::next(CandidateSet.begin(), InitialSize);
+         It != CandidateSet.end(); ++It) {
+      OverloadCandidate &Ovl = *It;
+      Ovl.RewrittenOpKind = Kind;
+    }
+  };
+
+  // If we have a relational or equality operation, add the rewritten candidates
+  // of the form: (LHS <=> RHS) @ 0
+  if (IsRelationalOrEquality)
+   AddCandidates(InputArgs, ROC_Rewritten);
+
+  // TODO: We should be able to avoid adding synthesized candidates when LHS and
+  // RHS have the same type, since the synthesized candidates for <=> should be
+  // the same as the rewritten ones. Note: It's still possible for the result
+  // of operator<=> to be usable only on the left or right side of the
+  // expression (0 @ <result>) or (<result> @ 0).
+
+  // For relational, equality, and three-way comparisons, add the rewritten and
+  // synthesized candidates of the form: 0 @ (RHS <=> LHS)
+  SmallVector<Expr *, 2> ReverseArgs(InputArgs.rbegin(), InputArgs.rend());
+  AddCandidates(ReverseArgs, ROC_Synthesized);
+}
+
 /// \brief Add function candidates found via argument-dependent lookup
 /// to the set of overloading candidates.
 ///
 /// This routine performs argument-dependent name lookup based on the
 /// given function name (which may also be an operator name) and adds
 /// all of the overload candidates found by ADL to the overload
 /// candidate set (C++ [basic.lookup.argdep]).
-void
-Sema::AddArgumentDependentLookupCandidates(DeclarationName Name,
-                                           SourceLocation Loc,
-                                           ArrayRef<Expr *> Args,
+void Sema::AddArgumentDependentLookupCandidates(
+    DeclarationName Name, SourceLocation Loc, ArrayRef<Expr *> Args,
     TemplateArgumentListInfo *ExplicitTemplateArgs,
-                                           OverloadCandidateSet& CandidateSet,
-                                           bool PartialOverloading) {
+    OverloadCandidateSet &CandidateSet, bool PartialOverloading) {
   ADLResult Fns;
 
   // FIXME: This approach for uniquing ADL results (and removing
   // redundant candidates from the set) relies on pointer-equality,
   // which means we need to key off the canonical decl.  However,
   // always going back to the canonical decl might not get us the
   // right set of default arguments.  What default arguments are
   // we supposed to consider on ADL candidates, anyway?

   };
 
   // Define functions that don't require ill-formed conversions for a given
   // argument to be better candidates than functions that do.
   unsigned NumArgs = Cand1.Conversions.size();
   assert(Cand2.Conversions.size() == NumArgs && "Overload candidate mismatch");
   bool HasBetterConversion = false;
   for (unsigned ArgIdx = StartArg; ArgIdx < NumArgs; ++ArgIdx) {
-    bool Cand1Bad = IsIllFormedConversion(Cand1.Conversions[ArgIdx]);
-    bool Cand2Bad = IsIllFormedConversion(Cand2.Conversions[ArgIdx]);
+    bool Cand1Bad = IsIllFormedConversion(Cand1.getConversion(ArgIdx));
+    bool Cand2Bad = IsIllFormedConversion(Cand2.getConversion(ArgIdx));
     if (Cand1Bad != Cand2Bad) {
       if (Cand1Bad)
         return false;
       HasBetterConversion = true;
     }
   }
 
   if (HasBetterConversion)
     return true;
 
   // C++ [over.match.best]p1:
   //   A viable function F1 is defined to be a better function than another
   //   viable function F2 if for all arguments i, ICSi(F1) is not a worse
   //   conversion sequence than ICSi(F2), and then...
   for (unsigned ArgIdx = StartArg; ArgIdx < NumArgs; ++ArgIdx) {
-    switch (CompareImplicitConversionSequences(S, Loc,
-                                               Cand1.Conversions[ArgIdx],
-                                               Cand2.Conversions[ArgIdx])) {
+    switch (CompareImplicitConversionSequences(
+        S, Loc, Cand1.getConversion(ArgIdx), Cand2.getConversion(ArgIdx))) {
     case ImplicitConversionSequence::Better:
       // Cand1 has a better conversion sequence.
       HasBetterConversion = true;
       break;
 
     case ImplicitConversionSequence::Worse:
       // Cand1 can't be better than Cand2.
       return false;

     return isa<CXXConstructorDecl>(Cand1.Function);
 
   //    -- F1 is a non-template function and F2 is a function template
   //       specialization, or, if not that,
   bool Cand1IsSpecialization = Cand1.Function &&
                                Cand1.Function->getPrimaryTemplate();
   bool Cand2IsSpecialization = Cand2.Function &&
                                Cand2.Function->getPrimaryTemplate();
   if (Cand1IsSpecialization != Cand2IsSpecialization)
     return Cand2IsSpecialization;
 
   //   -- F1 and F2 are function template specializations, and the function
   //      template for F1 is more specialized than the template for F2
   //      according to the partial ordering rules described in 14.5.5.2, or,
   //      if not that,
   if (Cand1IsSpecialization && Cand2IsSpecialization) {
     if (FunctionTemplateDecl *BetterTemplate

     auto *Cand2Class = cast<CXXRecordDecl>(Cand2.Function->getDeclContext());
     if (Cand1Class->isDerivedFrom(Cand2Class))
       return true;
     if (Cand2Class->isDerivedFrom(Cand1Class))
       return false;
     // Inherited from sibling base classes: still ambiguous.
   }
 
+  // Check C++2a tie-breakers for rewritten candidates
+  {
+    // --- F2 is a rewritten candidate ([over.match.oper]) and F1 is not.
+    if (Cand2.getRewrittenKind() && !Cand1.getRewrittenKind())
+      return true;
+    if (Cand1.getRewrittenKind() && Cand2.getRewrittenKind() &&
+        Cand2.getRewrittenKind() == ROC_Synthesized &&
+        Cand1.getRewrittenKind() != ROC_Synthesized) {
+      assert(StartArg == 0);
+      assert(NumArgs == 2);
+      // --- F1 and F2 are rewritten candidates, and F2 is a synthesized
+      // candidate with reversed order of parameters and F1 is not.
+      bool Matches = true;
+      for (unsigned ArgIdx = 0; ArgIdx < NumArgs; ++ArgIdx) {
+        if (Cand1.getParamType(ArgIdx).getCanonicalType() !=
+            Cand2.getParamType(ArgIdx).getCanonicalType()) {
+          Matches = false;
+          break;
+        }
+      }
+      if (Matches)
+        return true;
+    }
+  }
+
   // Check C++17 tie-breakers for deduction guides.
   {
     auto *Guide1 = dyn_cast_or_null<CXXDeductionGuideDecl>(Cand1.Function);
     auto *Guide2 = dyn_cast_or_null<CXXDeductionGuideDecl>(Cand2.Function);
     if (Guide1 && Guide2) {
       //  -- F1 is generated from a deduction-guide and F2 is not
       if (Guide1->isImplicit() != Guide2->isImplicit())
         return Guide2->isImplicit();

   }
 
   // Either we found no viable overloaded operator or we matched a
   // built-in operator. In either case, fall through to trying to
   // build a built-in operation.
   return CreateBuiltinUnaryOp(OpLoc, Opc, Input);
 }
 
+static ExprResult buildBinaryOperatorCandidate(Sema &S, SourceLocation OpLoc,
+                                               BinaryOperatorKind Opc,
+                                               const OverloadCandidate &Ovl,
+                                               Expr *LHSE, Expr *RHSE,
+                                               bool HadMultipleCandidates) {
+  Expr *Args[2] = {LHSE, RHSE};
+  OverloadedOperatorKind Op = BinaryOperator::getOverloadedOperator(Opc);
+  // We found a built-in operator or an overloaded operator.
+  FunctionDecl *FnDecl = Ovl.Function;
+
+  if (FnDecl) {
+    Expr *Base = nullptr;
+    // We matched an overloaded operator. Build a call to that
+    // operator.
+
+    // Convert the arguments.
+    if (CXXMethodDecl *Method = dyn_cast<CXXMethodDecl>(FnDecl)) {
+      // Ovl.Access is only meaningful for class members.
+      S.CheckMemberOperatorAccess(OpLoc, Args[0], Args[1], Ovl.FoundDecl);
+
+      ExprResult Arg1 =
+          S.PerformCopyInitialization(InitializedEntity::InitializeParameter(
+                                          S.Context, FnDecl->getParamDecl(0)),
+                                      SourceLocation(), Args[1]);
+      if (Arg1.isInvalid())
+        return ExprError();
+
+      ExprResult Arg0 = S.PerformObjectArgumentInitialization(
+          Args[0], /*Qualifier=*/nullptr, Ovl.FoundDecl, Method);
+      if (Arg0.isInvalid())
+        return ExprError();
+      Base = Args[0] = Arg0.getAs<Expr>();
+      Args[1] = Arg1.getAs<Expr>();
+    } else {
+      // Convert the arguments.
+      ExprResult Arg0 =
+          S.PerformCopyInitialization(InitializedEntity::InitializeParameter(
+                                          S.Context, FnDecl->getParamDecl(0)),
+                                      SourceLocation(), Args[0]);
+      if (Arg0.isInvalid())
+        return ExprError();
+
+      ExprResult Arg1 =
+          S.PerformCopyInitialization(InitializedEntity::InitializeParameter(
+                                          S.Context, FnDecl->getParamDecl(1)),
+                                      SourceLocation(), Args[1]);
+      if (Arg1.isInvalid())
+        return ExprError();
+      Args[0] = Arg0.getAs<Expr>();
+      Args[1] = Arg1.getAs<Expr>();
+    }
+
+    // Build the actual expression node.
+    ExprResult FnExpr = CreateFunctionRefExpr(S, FnDecl, Ovl.FoundDecl, Base,
+                                              HadMultipleCandidates, OpLoc);
+    if (FnExpr.isInvalid())
+      return ExprError();
+
+    // Determine the result type.
+    QualType ResultTy = FnDecl->getReturnType();
+    ExprValueKind VK = Expr::getValueKindForType(ResultTy);
+    ResultTy = ResultTy.getNonLValueExprType(S.Context);
+
+    CXXOperatorCallExpr *TheCall = new (S.Context) CXXOperatorCallExpr(
+        S.Context, Op, FnExpr.get(), Args, ResultTy, VK, OpLoc, S.FPFeatures);
+
+    if (S.CheckCallReturnType(FnDecl->getReturnType(), OpLoc, TheCall, FnDecl))
+      return ExprError();
+
+    ArrayRef<const Expr *> ArgsArray(Args, 2);
+    const Expr *ImplicitThis = nullptr;
+    // Cut off the implicit 'this'.
+    if (isa<CXXMethodDecl>(FnDecl)) {
+      ImplicitThis = ArgsArray[0];
+      ArgsArray = ArgsArray.slice(1);
+    }
+
+    // Check for a self move.
+    if (Op == OO_Equal)
+      S.DiagnoseSelfMove(Args[0], Args[1], OpLoc);
+
+    S.checkCall(FnDecl, nullptr, ImplicitThis, ArgsArray,
+                isa<CXXMethodDecl>(FnDecl), OpLoc, TheCall->getSourceRange(),
+                Sema::VariadicDoesNotApply);
+
+    return S.MaybeBindToTemporary(TheCall);
+
+  } else {
+    // We matched a built-in operator. Convert the arguments, then
+    // break out so that we will build the appropriate built-in
+    // operator node.
+    ExprResult ArgsRes0 =
+        S.PerformImplicitConversion(Args[0], Ovl.BuiltinParamTypes[0],
+                                    Ovl.Conversions[0], Sema::AA_Passing);
+    if (ArgsRes0.isInvalid())
+      return ExprError();
+    Args[0] = ArgsRes0.get();
+
+    ExprResult ArgsRes1 =
+        S.PerformImplicitConversion(Args[1], Ovl.BuiltinParamTypes[1],
+                                    Ovl.Conversions[1], Sema::AA_Passing);
+    if (ArgsRes1.isInvalid())
+      return ExprError();
+    Args[1] = ArgsRes1.get();
+  }
+  // We matched a built-in operator; build it.
+  return S.CreateBuiltinBinOp(OpLoc, Opc, Args[0], Args[1]);
+}
+
+namespace {
+
+/// \brief RewrittenCandidateOverloadResolver - This class handles initial
+/// overload resolution for candidate sets which include rewritten candidates.
+///
+/// Rewritten candidates haven't been fully checked for validity. They may still
+/// be invalid if:
+///    (A) The rewritten candidate is a builtin, but semantic checking of the
+///        builtin would fail.
+///    (B) The result of the "partially rewritten expression"
+///        (ie the (LHS <=> RHS) part) is ill-formed when used as an operand to
+///        (<result> @ 0) or (0 @ <result>).
+///
+/// TODO: Separate out the bits of semantic checking for builtin spaceship
+/// operators which determine validity and the return type, and use that instead
+/// of building the full expression to check validity.
+class RewrittenCandidateOverloadResolver {
+public:
+  RewrittenCandidateOverloadResolver(Sema &S, SourceLocation OpLoc,
+                                     BinaryOperatorKind Opc,
+                                     ArrayRef<Expr *> Args,
+                                     const UnresolvedSetImpl &Fns,
+                                     bool PerformADL, OverloadCandidateSet &CS)
+      : S(S), OpLoc(OpLoc), Opc(Opc), Args(Args), Fns(Fns),
+        PerformADL(PerformADL), CandidateSet(CS) {}
+
+  ExprResult ResolveRewrittenCandidates() {
+    ExprResult FinalResult = ExprError();
+    OverloadCandidateSet::iterator Best;
+    OverloadingResult OvlRes;
+    do {
+      OvlRes = CandidateSet.BestViableFunction(S, OpLoc, Best);
+    } while (RemoveNonViableRewrittenCandidates(OvlRes, Best, FinalResult));
+    return FinalResult;
+  }
+private:
+  bool RemoveNonViableRewrittenCandidates(OverloadingResult OvlRes,
+                                          OverloadCandidateSet::iterator Best,
+                                          ExprResult &FinalResult);
+  ExprResult BuildRewrittenCandidate(const OverloadCandidate &Ovl);
+
+  RewrittenCandidateOverloadResolver(
+      RewrittenCandidateOverloadResolver const &) = delete;
+  RewrittenCandidateOverloadResolver &
+  operator=(RewrittenCandidateOverloadResolver const &) = delete;
+
+private:
+  Sema &S;
+  SourceLocation OpLoc;
+  BinaryOperatorKind Opc;
+  ArrayRef<Expr *> Args;
+  const UnresolvedSetImpl &Fns;
+  bool PerformADL;
+  OverloadCandidateSet &CandidateSet;
+};
+} // end namespace
+
+
+ExprResult RewrittenCandidateOverloadResolver::BuildRewrittenCandidate(
+    const OverloadCandidate &Ovl) {
+  Expr *RewrittenArgs[2] = {Args[0], Args[1]};
+  bool IsSynthesized = Ovl.getRewrittenKind() == ROC_Synthesized;
+  if (IsSynthesized)
+    std::swap(RewrittenArgs[0], RewrittenArgs[1]);
+
+  // Supress diagnostics when building the expressions for the specified
+  // candidate. If evaluation fails the candidate will be marked non-viable
+  // and the best viable candidate re-computed.
+  Sema::TentativeAnalysisScope DiagnosticScopeGuard(S);
+
+  // Build the '(LHS <=> RHS)' operand to the full expression.
+  ExprResult RewrittenRes = buildBinaryOperatorCandidate(
+      S, OpLoc, BO_Cmp, Ovl, RewrittenArgs[0], RewrittenArgs[1],
+      /*HadMultipleCandidates*/ false);
+  if (RewrittenRes.isInvalid())
+    return ExprError();
+
+  // Now attempt to build the full expression '(LHS <=> RHS) @ 0' using the
+  // evaluated operand and the literal 0.
+  llvm::APInt I =
+      llvm::APInt::getNullValue(S.Context.getIntWidth(S.Context.IntTy));
+  Expr *Zero =
+      IntegerLiteral::Create(S.Context, I, S.Context.IntTy, SourceLocation());
+
+  Expr *NewLHS = RewrittenRes.get();
+  Expr *NewRHS = Zero;
+  if (Ovl.getRewrittenKind() == ROC_Synthesized)
+    std::swap(NewLHS, NewRHS);
+
+  return S.CreateOverloadedBinOp(OpLoc, Opc, Fns, NewLHS, NewRHS, PerformADL,
+                                 /*AllowRewrittenCandidates*/ false);
+}
+
+/// Rewritten candidates have been added but not checked for validity. They
+/// could still be non-viable if:
+///  (A) The rewritten call (x <=> y) is a builtin, but it will be ill-formed
+///      when built (for example it has narrowing conversions).
+///  (B) The expression (x <=> y) @ 0 is ill-formed for the result of (x <=> y).
+///
+/// If either is the case, this function should be considered non-viable and
+/// another best viable function needs to be computed.
+///
+/// Therefore, we do the following:
+///  (1) If we have no viable candidate, or a deleted candidate, stop.
+///      Otherwise, if the best-viable-candidate or a set of ambiguous
+///      candidates, and none of which are rewritten, stop.
+///
+///  (2) If the best viable candidate is a rewritten candidate, build and
+///      check the full expression for that candidate. If it succeeds return
+///      that result. Otherwise, mark the candidate as non-viable, re-compute
+///      the best viable function, and continue.
+///
+///  (3) If we have ambiguity order the set of viable candidates. For each
+///      rewritten candidate causing ambiguity:
+///
+///        (3.1) build the full expression for the specified candidate.
+///        (3.2) If the result is invalid, mark the candidate as non-viable.
+///        (3.3) Otherwise, cache the valid result for later.
+///
+///      If any of the rewritten candidates were marked non-viable, recompute
+///      the best viable function and continue. Otherwise, stop since the
+///      lookup is still ambiguous,
+bool RewrittenCandidateOverloadResolver::RemoveNonViableRewrittenCandidates(
+    OverloadingResult OvlRes, OverloadCandidateSet::iterator Best,
+    ExprResult &FinalResult) {
+  switch (OvlRes) {
+  case OR_Deleted:
+    // FIXME(EricWF): If we've found a deleted rewritten operator, it's
+    // possible we should have never considered it a viable candidate.
+  case OR_No_Viable_Function:
+    return false;
+
+  case OR_Success: {
+    OverloadCandidate &Ovl = *Best;
+    if (!Ovl.getRewrittenKind())
+      return false;
+    // Build the full expression for the rewritten candidate, and return it if
+    // it's valid. Otherwise mork this candidate as non-viable and continue.
+    ExprResult Res = BuildRewrittenCandidate(Ovl);
+    if (Res.isInvalid()) {
+      Ovl.Viable = false;
+      return true;
+    }
+    FinalResult = Res;
+    return false;
+  }
+  case OR_Ambiguous: {
+    SmallVector<OverloadCandidate *, 16> Overloads;
+    // Gather all viable candidates. If non of the viable candidates are
+    // rewritten, stop.
+    bool HasRewritten = false;
+    for (auto &Ovl : CandidateSet) {
+      if (!Ovl.Viable)
+        continue;
+      HasRewritten |= Ovl.getRewrittenKind();
+      Overloads.push_back(&Ovl);
+    }
+    if (!HasRewritten)
+      return false;
+    auto CmpOverloads = [&](const OverloadCandidate *C1,
+                            const OverloadCandidate *C2) {
+      return isBetterOverloadCandidate(S, *C1, *C2, CandidateSet.getLocation(),
+                                       CandidateSet.getKind());
+    };
+    // Sort the candidate functions based on their partial ordering,
+    // and find the first N functions which rank equally.
+    std::sort(Overloads.begin(), Overloads.end(), CmpOverloads);
+    auto End = std::lower_bound(Overloads.begin(), Overloads.end(),
+                                Overloads[0], CmpOverloads);
+
+    int NumViableCandidates = 0;
+    ExprResult ViableRewritten = ExprError();
+    for (auto It = Overloads.begin(); It != End; ++It) {
+      OverloadCandidate &Ovl = **It;
+      if (Ovl.getRewrittenKind()) {
+        ExprResult Res = BuildRewrittenCandidate(Ovl);
+        if (Res.isInvalid()) {
+          Ovl.Viable = false;
+          continue;
+        }
+        ViableRewritten = Res;
+      }
+      ++NumViableCandidates;
+    }
+    // If only one of the candidates turns out to be viable, and it's a
+    // rewritten candidate, return that candidate as the result.
+    if (NumViableCandidates == 1 && !ViableRewritten.isInvalid()) {
+      FinalResult = ViableRewritten;
+      return false;
+    }
+    // If none of the rewritten
+    if (NumViableCandidates > 1)
+      return false;
+    return true;
+  }
+  }
+  llvm_unreachable("unhandled case");
+}
+
 /// \brief Create a binary operation that may resolve to an overloaded
 /// operator.
 ///
 /// \param OpLoc The location of the operator itself (e.g., '+').
 ///
 /// \param Opc The BinaryOperatorKind that describes this operator.
 ///
 /// \param Fns The set of non-member functions that will be
 /// considered by overload resolution. The caller needs to build this
 /// set based on the context using, e.g.,
 /// LookupOverloadedOperatorName() and ArgumentDependentLookup(). This
 /// set should not contain any member functions; those will be added
 /// by CreateOverloadedBinOp().
 ///
 /// \param LHS Left-hand argument.
 /// \param RHS Right-hand argument.
-ExprResult
-Sema::CreateOverloadedBinOp(SourceLocation OpLoc,
+ExprResult Sema::CreateOverloadedBinOp(SourceLocation OpLoc,
                                        BinaryOperatorKind Opc,
-                            const UnresolvedSetImpl &Fns,
-                            Expr *LHS, Expr *RHS, bool PerformADL) {
+                                       const UnresolvedSetImpl &Fns, Expr *LHS,
+                                       Expr *RHS, bool PerformADL,
+                                       bool AllowRewrittenCandidates) {
   Expr *Args[2] = { LHS, RHS };
   LHS=RHS=nullptr; // Please use only Args instead of LHS/RHS couple
 
   OverloadedOperatorKind Op = BinaryOperator::getOverloadedOperator(Opc);
   DeclarationName OpName = Context.DeclarationNames.getCXXOperatorName(Op);
 
   // If either side is type-dependent, create an appropriate dependent
   // expression.

   if (Opc != BO_Assign && PerformADL)
     AddArgumentDependentLookupCandidates(OpName, OpLoc, Args,
                                          /*ExplicitTemplateArgs*/ nullptr,
                                          CandidateSet);
 
   // Add builtin operator candidates.
   AddBuiltinOperatorCandidates(Op, OpLoc, Args, CandidateSet);
 
-  bool HadMultipleCandidates = (CandidateSet.size() > 1);
+  bool BeforeRewrittenSize = CandidateSet.size();
+  // C++2a Add rewritten and synthesized operator candidates.
+  if (getLangOpts().CPlusPlus2a && AllowRewrittenCandidates)
+    AddRewrittenOperatorCandidates(Op, OpLoc, Args, CandidateSet, PerformADL);
 
-  // Perform overload resolution.
-  OverloadCandidateSet::iterator Best;
-  switch (CandidateSet.BestViableFunction(*this, OpLoc, Best)) {
-    case OR_Success: {
-      // We found a built-in operator or an overloaded operator.
-      FunctionDecl *FnDecl = Best->Function;
-
-      if (FnDecl) {
-        Expr *Base = nullptr;
-        // We matched an overloaded operator. Build a call to that
-        // operator.
+  bool HasRewrittenCandidates = BeforeRewrittenSize != CandidateSet.size();
 
-        // Convert the arguments.
-        if (CXXMethodDecl *Method = dyn_cast<CXXMethodDecl>(FnDecl)) {
-          // Best->Access is only meaningful for class members.
-          CheckMemberOperatorAccess(OpLoc, Args[0], Args[1], Best->FoundDecl);
+  if (HasRewrittenCandidates) {
+    RewrittenCandidateOverloadResolver RewrittenOvlResolver(
+        *this, OpLoc, Opc, Args, Fns, PerformADL, CandidateSet);
 
-          ExprResult Arg1 =
-            PerformCopyInitialization(
-              InitializedEntity::InitializeParameter(Context,
-                                                     FnDecl->getParamDecl(0)),
-              SourceLocation(), Args[1]);
-          if (Arg1.isInvalid())
-            return ExprError();
+    // Perform initial overload resolution that includes partially checked
+    // rewritten candidates, removing rewritten candidates which turn out to be
+    // invalid as needed.
+    ExprResult RewrittenResult =
+        RewrittenOvlResolver.ResolveRewrittenCandidates();
 
-          ExprResult Arg0 =
-            PerformObjectArgumentInitialization(Args[0], /*Qualifier=*/nullptr,
-                                                Best->FoundDecl, Method);
-          if (Arg0.isInvalid())
-            return ExprError();
-          Base = Args[0] = Arg0.getAs<Expr>();
-          Args[1] = RHS = Arg1.getAs<Expr>();
-        } else {
-          // Convert the arguments.
-          ExprResult Arg0 = PerformCopyInitialization(
-            InitializedEntity::InitializeParameter(Context,
-                                                   FnDecl->getParamDecl(0)),
-            SourceLocation(), Args[0]);
-          if (Arg0.isInvalid())
-            return ExprError();
-
-          ExprResult Arg1 =
-            PerformCopyInitialization(
-              InitializedEntity::InitializeParameter(Context,
-                                                     FnDecl->getParamDecl(1)),
-              SourceLocation(), Args[1]);
-          if (Arg1.isInvalid())
-            return ExprError();
-          Args[0] = LHS = Arg0.getAs<Expr>();
-          Args[1] = RHS = Arg1.getAs<Expr>();
-        }
-
-        // Build the actual expression node.
-        ExprResult FnExpr = CreateFunctionRefExpr(*this, FnDecl,
-                                                  Best->FoundDecl, Base,
-                                                  HadMultipleCandidates, OpLoc);
-        if (FnExpr.isInvalid())
-          return ExprError();
-
-        // Determine the result type.
-        QualType ResultTy = FnDecl->getReturnType();
-        ExprValueKind VK = Expr::getValueKindForType(ResultTy);
-        ResultTy = ResultTy.getNonLValueExprType(Context);
-
-        CXXOperatorCallExpr *TheCall =
-          new (Context) CXXOperatorCallExpr(Context, Op, FnExpr.get(),
-                                            Args, ResultTy, VK, OpLoc,
-                                            FPFeatures);
-
-        if (CheckCallReturnType(FnDecl->getReturnType(), OpLoc, TheCall,
-                                FnDecl))
-          return ExprError();
-
-        ArrayRef<const Expr *> ArgsArray(Args, 2);
-        const Expr *ImplicitThis = nullptr;
-        // Cut off the implicit 'this'.
-        if (isa<CXXMethodDecl>(FnDecl)) {
-          ImplicitThis = ArgsArray[0];
-          ArgsArray = ArgsArray.slice(1);
-        }
-
-        // Check for a self move.
-        if (Op == OO_Equal)
-          DiagnoseSelfMove(Args[0], Args[1], OpLoc);
-
-        checkCall(FnDecl, nullptr, ImplicitThis, ArgsArray,
-                  isa<CXXMethodDecl>(FnDecl), OpLoc, TheCall->getSourceRange(),
-                  VariadicDoesNotApply);
-
-        return MaybeBindToTemporary(TheCall);
-      } else {
-        // We matched a built-in operator. Convert the arguments, then
-        // break out so that we will build the appropriate built-in
-        // operator node.
-        ExprResult ArgsRes0 =
-            PerformImplicitConversion(Args[0], Best->BuiltinParamTypes[0],
-                                      Best->Conversions[0], AA_Passing);
-        if (ArgsRes0.isInvalid())
-          return ExprError();
-        Args[0] = ArgsRes0.get();
-
-        ExprResult ArgsRes1 =
-            PerformImplicitConversion(Args[1], Best->BuiltinParamTypes[1],
-                                      Best->Conversions[1], AA_Passing);
-        if (ArgsRes1.isInvalid())
-          return ExprError();
-        Args[1] = ArgsRes1.get();
-        break;
-      }
+    // If overload resolution was successful and the result was a re-written
+    // overload candidate, then that candidate was evaluated and we can return
+    // that value directly.
+    if (!RewrittenResult.isInvalid())
+      return RewrittenResult;
   }
 
+  // Perform final overload resolution.
+  bool HadMultipleCandidates = (CandidateSet.size() > 1);
+  OverloadCandidateSet::iterator Best;
+  switch (CandidateSet.BestViableFunction(*this, OpLoc, Best)) {
+  case OR_Success:
+    assert(
+        !Best->getRewrittenKind() &&
+        "rewritten candidates should have already been resolved and evaluated");
+    return buildBinaryOperatorCandidate(*this, OpLoc, Opc, *Best, Args[0],
+                                        Args[1], HadMultipleCandidates);
   case OR_No_Viable_Function: {
     // C++ [over.match.oper]p9:
     //   If the operator is the operator , [...] and there are no
     //   viable functions, then the operator is assumed to be the
     //   built-in operator and interpreted according to clause 5.
     if (Opc == BO_Comma)
       break;
 
     // For class as left operand for assignment or compound assignment
     // operator do not fall through to handling in built-in, but report that
     // no overloaded assignment operator found
     ExprResult Result = ExprError();
-      if (Args[0]->getType()->isRecordType() &&
-          Opc >= BO_Assign && Opc <= BO_OrAssign) {
+    if (Args[0]->getType()->isRecordType() && Opc >= BO_Assign &&
+        Opc <= BO_OrAssign) {
       Diag(OpLoc, diag::err_ovl_no_viable_oper)
-             << BinaryOperator::getOpcodeStr(Opc)
-             << Args[0]->getSourceRange() << Args[1]->getSourceRange();
+          << BinaryOperator::getOpcodeStr(Opc) << Args[0]->getSourceRange()
+          << Args[1]->getSourceRange();
       if (Args[0]->getType()->isIncompleteType()) {
         Diag(OpLoc, diag::note_assign_lhs_incomplete)
-            << Args[0]->getType()
-            << Args[0]->getSourceRange() << Args[1]->getSourceRange();
+            << Args[0]->getType() << Args[0]->getSourceRange()
+            << Args[1]->getSourceRange();
       }
     } else {
       // This is an erroneous use of an operator which can be overloaded by
       // a non-member function. Check for non-member operators which were
       // defined too late to be candidates.
       if (DiagnoseTwoPhaseOperatorLookup(*this, Op, OpLoc, Args))
         // FIXME: Recover by calling the found function.
         return ExprError();
 
       // No viable function; try to create a built-in operation, which will
       // produce an error. Then, show the non-viable candidates.
       Result = CreateBuiltinBinOp(OpLoc, Opc, Args[0], Args[1]);
     }
     assert(Result.isInvalid() &&
            "C++ binary operator overloading is missing candidates!");
     if (Result.isInvalid())
       CandidateSet.NoteCandidates(*this, OCD_AllCandidates, Args,
                                   BinaryOperator::getOpcodeStr(Opc), OpLoc);
     return Result;
   }
-
   case OR_Ambiguous:
     Diag(OpLoc, diag::err_ovl_ambiguous_oper_binary)
-          << BinaryOperator::getOpcodeStr(Opc)
-          << Args[0]->getType() << Args[1]->getType()
-          << Args[0]->getSourceRange() << Args[1]->getSourceRange();
+        << BinaryOperator::getOpcodeStr(Opc) << Args[0]->getType()
+        << Args[1]->getType() << Args[0]->getSourceRange()
+        << Args[1]->getSourceRange();
     CandidateSet.NoteCandidates(*this, OCD_ViableCandidates, Args,
                                 BinaryOperator::getOpcodeStr(Opc), OpLoc);
     return ExprError();
 
   case OR_Deleted:
     if (isImplicitlyDeleted(Best->Function)) {
       CXXMethodDecl *Method = cast<CXXMethodDecl>(Best->Function);
       Diag(OpLoc, diag::err_ovl_deleted_special_oper)
           << Context.getRecordType(Method->getParent())
           << getSpecialMember(Method);
 
       // The user probably meant to call this special member. Just
       // explain why it's deleted.
       NoteDeletedFunction(Method);
       return ExprError();
     } else {
       Diag(OpLoc, diag::err_ovl_deleted_oper)
-          << Best->Function->isDeleted()
-          << BinaryOperator::getOpcodeStr(Opc)
+          << Best->Function->isDeleted() << BinaryOperator::getOpcodeStr(Opc)
           << getDeletedOrUnavailableSuffix(Best->Function)
           << Args[0]->getSourceRange() << Args[1]->getSourceRange();
     }
     CandidateSet.NoteCandidates(*this, OCD_AllCandidates, Args,
                                 BinaryOperator::getOpcodeStr(Opc), OpLoc);
     return ExprError();
   }
 

test/SemaCXX/compare-cxx2a.cpp

void test_enum_enum_compare_no_builtin() {
   ASSERT_EXPR_TYPE(r1, std::strong_ordering);
   (void)(EnumA::A <=> EnumA::A); // expected-warning {{self-comparison always evaluates to 'std::strong_ordering::equal'}}
   (void)(EnumA::A <=> EnumB::B); // expected-error {{invalid operands to binary expression ('EnumCompareTests::EnumA' and 'EnumCompareTests::EnumB')}}
   (void)(EnumB::B <=> EnumA::A); // expected-error {{invalid operands}}
 }
 
 template <int>
 struct Tag {};
-// expected-note@+1 {{candidate}}
-Tag<0> operator<=>(EnumA, EnumA) {
-  return {};
+std::strong_ordering operator<=>(EnumA, EnumA) {
+  return std::strong_ordering::equal;
 }
-Tag<1> operator<=>(EnumA, EnumB) {
-  return {};
+// expected-note@+1 {{candidate function}},
+std::strong_ordering operator<=>(EnumA a, EnumB b) {
+  return ((int)a <=> (int)b);
 }
 
 void test_enum_ovl_provided() {
   auto r1 = (EnumA::A <=> EnumA::A);
-  ASSERT_EXPR_TYPE(r1, Tag<0>);
+  ASSERT_EXPR_TYPE(r1, std::strong_ordering);
   auto r2 = (EnumA::A <=> EnumB::B);
-  ASSERT_EXPR_TYPE(r2, Tag<1>);
-  (void)(EnumB::B <=> EnumA::A); // expected-error {{invalid operands to binary expression ('EnumCompareTests::EnumB' and 'EnumCompareTests::EnumA')}}
+  ASSERT_EXPR_TYPE(r2, std::strong_ordering);
+  (void)(EnumB::B <=> EnumA::A); // OK, chooses reverse order synthesized candidate.
+  (void)(EnumB::B <=> EnumC::C); // expected-error {{invalid operands to binary expression ('EnumCompareTests::EnumB' and 'EnumCompareTests::EnumC')}}
 }
 
 void enum_float_test() {
   enum EnumA { A };
   (void)(A <=> (float)0);       // expected-error {{invalid operands to binary expression ('EnumA' and 'float')}}
   (void)((double)0 <=> A);      // expected-error {{invalid operands to binary expression ('double' and 'EnumA')}}
   (void)((long double)0 <=> A); // expected-error {{invalid operands to binary expression ('long double' and 'EnumA')}}
 }
void test_mixed_float_int(float f, double d, long double ld) {
 
   auto r3 = (s <=> f);
   ASSERT_EXPR_TYPE(r3, std::partial_ordering);
 
   auto r4 = (0.0 <=> i);
   ASSERT_EXPR_TYPE(r4, std::partial_ordering);
 
 }
+
+namespace TestRewritting {
+
+struct T {
+  int x;
+  // expected-note@+1 {{candidate}}
+  constexpr std::strong_ordering operator<=>(T y) const {
+    return (x <=> y.x);
+  }
+};
+
+struct U {
+  int x;
+  // FIXME: This diagnostic is terrible.
+  // expected-note@+1 {{candidate function not viable: requires single argument 'y', but 2 arguments were provided}}
+  constexpr std::strong_equality operator<=>(T y) const {
+    if (x == y.x)
+      return std::strong_equality::equal;
+    return std::strong_equality::nonequal;
+  }
+};
+
+struct X { int x; };
+struct Y { int x; };
+struct Tag {};
+// expected-note@+1 2 {{candidate}}
+Tag operator<=>(X, Y) {
+  return {};
+}
+// expected-note@+1 2 {{candidate}}
+constexpr auto operator<=>(Y y, X x) {
+  return y.x <=> x.x;
+}
+
+void foo() {
+  T t{42};
+  T t2{0};
+  U u{101};
+  auto r1 = (t <=> u);
+  ASSERT_EXPR_TYPE(r1, std::strong_equality);
+  auto r2 = (t <=> t2);
+  ASSERT_EXPR_TYPE(r2, std::strong_ordering);
+
+  auto r3 = t == u;
+  ASSERT_EXPR_TYPE(r3, bool);
+
+  (void)(t < u); // expected-error {{invalid operands to binary expression ('TestRewritting::T' and 'TestRewritting::U')}}
+
+  constexpr X x{1};
+  constexpr Y y{2};
+  constexpr auto r4 = (y < x);
+  static_assert(r4 == false);
+  constexpr auto r5 = (x < y);
+  static_assert(r5 == true);
+}
+
+} // namespace TestRewritting