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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
