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

[Eric Fiselier via Phabricator via cfe-commits]

EricWF marked 2 inline comments as done.
EricWF added inline comments.


================
Comment at: lib/Sema/SemaOverload.cpp:12543-12547
+  Expr *Original = new (S.Context)
+      BinaryOperator(OpaqueValueExpr::Create(S.Context, Args[0]),
+                     OpaqueValueExpr::Create(S.Context, Args[1]), Opc,
+                     Rewritten->getType(), Rewritten->getValueKind(),
+                     Rewritten->getObjectKind(), OpLoc, S.FPFeatures);
----------------
EricWF wrote:
> rsmith wrote:
> > 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.)
> 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.
Nevermind. This was answered offline.


https://reviews.llvm.org/D45680