[PATCH] D32155: [APInt] Use lshrInPlace to replace lshr where possible

[Hans Wennborg via Phabricator via llvm-commits]

hans added a comment.

Besides the `return *this` issue, this looks great!



================
Comment at: include/llvm/ADT/APInt.h:892
   /// Logical right-shift this APInt by ShiftAmt in place.
-  void lshrInPlace(unsigned ShiftAmt);
+  APInt &lshrInPlace(unsigned ShiftAmt);
 
----------------
RKSimon wrote:
> Other than operators we don't tend to use this pattern to return *this - is it a good idea to introduce it?
I'm also not sure about this one. While I can see that it would be convenient, it doesn't seem like it would apply in that many places, and it could potentially be a source of subtle bugs, i.e. the reader might not notice the the variable gets modified in-place when this is used as part of a larger expression.


https://reviews.llvm.org/D32155