[PATCH] D60510: [ADT] Fix template parameter names of llvm::{upper|lower}_bound

David Blaikie via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Apr 16 11:31:45 PDT 2019


dblaikie added a comment.

In D60510#1468366 <https://reviews.llvm.org/D60510#1468366>, @ilya-biryukov wrote:

> In D60510#1467262 <https://reviews.llvm.org/D60510#1467262>, @dblaikie wrote:
>
> > Might've made sense to separate the NFC renaming from the perfect forwarding change (& the perfect forwarding change could potentially have some matching test coverage?)
>
>
> I agree, sorry about mashing them in together. That said, I was confident this wouldn't cause any trouble in practice.
>  What kinds of tests are you thinking about? Checking that `llvm::lower_bound` works on non-copyable types?


Yeah, that'd probably suffice (if you were being extra super robust, and we generally aren't with these thin wrappers, I guess you could test that the parameter can be a temporary and a non-const value where the underlying type or passed in comparator takes by non-const ref (that would test that the forwarding works for things that must be passed by const ref and things that can't be passed by const ref and must be non-const ref))


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60510/new/

https://reviews.llvm.org/D60510





More information about the llvm-commits mailing list