leonardchan added inline comments.
================ Comment at: clang/lib/Basic/FixedPoint.cpp:129 + std::max(NonFractBits, OtherNonFractBits) + CommonScale; + + bool ResultIsSigned = isSigned() || Other.isSigned(); ---------------- ebevhan wrote: > Does this properly compensate for types of equal width but different > signedness? > > If you have a signed 8-bit 7-scale fixed point number, and operate with an > unsigned 8-bit integer, you'll get CommonWidth=15 and CommonScale=7, leaving > 8 bits of integral. But the MSB in that cannot both be sign bit and unsigned > MSB at the same time. I think you need an extra bit. Added test for this also. ================ Comment at: clang/lib/Sema/SemaExpr.cpp:1306 + OtherTy = ResultTy; + } + ---------------- ebevhan wrote: > Does this handle compound assignment? The other functions handle this > specially. Currently no. I was initially intending to add addition compound assignment in a different patch, but do you think it would be appropriate to include it here? I imagine the changes to Sema wouldn't be difficult, but would probably require adding a lot more tests. Repository: rC Clang https://reviews.llvm.org/D53738 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits