[PATCH] D32108: [APInt] Rename getSignBit to getSignMask

Sanjay Patel via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Apr 17 07:21:08 PDT 2017


spatel accepted this revision.
spatel added a comment.
This revision is now accepted and ready to land.

The first part of the plan (this patch) makes sense to me, so LGTM. I think I wrote some of the code that already uses a 'SignMask' variable, so it may be worth waiting to hear another opinion. :)

The second part does not make sense to me. If 'getSignBit' is ambiguous/confusing terminology today, then re-introducing the same name with a different meaning will also be confusing going forward. I'd make that 'isSignBitSet/isSignBitClear' to make it less confusing.


https://reviews.llvm.org/D32108





More information about the llvm-commits mailing list