cpplearner marked an inline comment as done.
cpplearner added inline comments.
================
Comment at: clang/lib/AST/ExprConstant.cpp:3441
+ Value);
+ }
APSInt LHS = HandleIntToIntCast(Info, E, PromotedLHSType,
----------------
rjmccall wrote:
> cpplearner wrote:
> > rjmccall wrote:
> > > Can we more fundamentally restructure this entire handler so that, if the
> > > compound assignment's computation result type is an arithmetic type, we
> > > just promote both operands to that type, do the arithmetic there, and
> > > then coerce back down? This is C++, so the LHS type imposes almost no
> > > restrictions on the RHS type; also, this is Clang, where we support way
> > > too many arithmetic types for our own good.
> > It seems the conditional statement is unavoidable, because `APSInt` and
> > `APFloat` can't be handled at the same time (i.e. you need to choose among
> > `Handle{Int,Float}To{Int,Float}Cast`, and between
> > `handleIntIntBinOp`/`handleFloatFloatBinOp`). Maybe it's possible to add a
> > layer that can accept both `APSInt` and `APFloat`, but it seems like an
> > overkill if it's only used in the compound assignment case.
> But we can have `HandleValueTo{Int,Float}Cast` functions that start with an
> arbitrary `APValue` and do the switch on that type, and we can have a
> `HandleValueValueBinOp` function that asserts that its operands have the same
> type and does the switch, and those two together should be good enough for
> what we need here.
That's what I mean by "add a layer", and I think it's unworthy.
Maybe it makes more sense as part of a larger scale rewrite, but that's more
than I can deal with.
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D55413/new/
https://reviews.llvm.org/D55413
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
