[PATCH] D53447: [adt] SparseBitVector::test() should be const

[Daniel Sanders via Phabricator via llvm-commits]

dsanders added a comment.

Thanks



================
Comment at: include/llvm/ADT/SparseBitVector.h:310
   }
+  ElementListConstIter FindLowerBoundConst(unsigned ElementIndex) const {
+    return FindLowerBoundImpl(ElementIndex);
----------------
rtereshin wrote:
> Technically, `const` qualifier is a part of the member function signature, so no need for a different name. If called from another  const function, like `test`, the const version of this function will be called regardless of the object itself being const or not.
> 
> But honestly, I like making the difference explicit in this specific case, it draws attention, and it's probably a good thing as `FindLowerBoundImpl` needs to be used or modified with care so we don't accidentally leak a non-const iterator from a const object in the future.
> 
> In the end, I'm fine with both, a different name and the same.
Yep, that's the reason I used separate names in this particular case. The const_casts are safe so long as we only call the const version in a const context and the non-const version in a non-const context. It's difficult to tell which you're calling when the names are the same.


Repository:
  rL LLVM

https://reviews.llvm.org/D53447