Refactor UsualArithmeticConversions()

Committed at 139033

[Sean Hunt, 2011-08-09 23:18:14]

I like the nature of this refactor, but some specific curns are highlighted.

http://codereview.appspot.com/4810092/diff/1/lib/Sema/SemaExpr.cpp
File lib/Sema/SemaExpr.cpp (right):

http://codereview.appspot.com/4810092/diff/1/lib/Sema/SemaExpr.cpp#newcode532
lib/Sema/SemaExpr.cpp:532: ExprResult &complexExpr,
For all these functions, it would be good to put something in the documentation
indicating the fact that these functions change arguments, and also that they
are a part of usual arithmetic conversions. This might be mistaken for a general
int -> complex float conversion otherwise. I'd suggest putting it in the name if
possible, but that might be tricky due to the length. Perhaps you could drop
'float' out of 'complex float'?

[rtrieu, 2011-08-10 22:23:42]

http://codereview.appspot.com/4810092/diff/1/lib/Sema/SemaExpr.cpp
File lib/Sema/SemaExpr.cpp (right):

http://codereview.appspot.com/4810092/diff/1/lib/Sema/SemaExpr.cpp#newcode532
lib/Sema/SemaExpr.cpp:532: ExprResult &complexExpr,
On 2011/08/09 23:18:15, Sean Hunt wrote:
> For all these functions, it would be good to put something in the
documentation
> indicating the fact that these functions change arguments, and also that they
> are a part of usual arithmetic conversions. This might be mistaken for a
general
> int -> complex float conversion otherwise. I'd suggest putting it in the name
if
> possible, but that might be tricky due to the length. Perhaps you could drop
> 'float' out of 'complex float'?

I added to the comments that these are helper functions to
UsualArithmeticConversions().  Dropping changing 'complex float' to just
'complex'  might get some confusion with the 'complex int' case further down.

I'm not sure why it is needed to point out that the arguments might be modified.
 They are passed in by reference as a ExprResult instead of as a Expr*.  Many of
the other functions that modify arguments also don't explicitly say they do so.