[libcxx-commits] [PATCH] D121074: [libc++] Implement std::boyer_moore{, _horspool}_searcher

[Louis Dionne via Phabricator via libcxx-commits]

ldionne accepted this revision.
ldionne added a comment.

LGTM but please fix the experimental version too since it's so trivial.



================
Comment at: libcxx/include/__functional/boyer_moore_searcher.h:208
+
+    vector<value_type> __scratch(__count);
+
----------------
adamdebreceni wrote:
> ldionne wrote:
> > adamdebreceni wrote:
> > > philnik wrote:
> > > > adamdebreceni wrote:
> > > > > shouldn't this be `vector<difference_type>`?
> > > > Why do you think it should be `vector<difference_type>`?
> > > if `value_type` is `signed` and the range consists of e.g. the same values with length more than the max, `__compute_bm_prefix` will cause a signed overflow, moreover as `__scratch` can now contain negative values we could index out of `__suffix_`
> > We should add a test and fix this bug in both this version and the `experimental/functional` version as well (even though we'll eventually remove that one).
> if we plan on fixing the `expirmental/functional`  we should add a `+1` to the size in the `_BMSkipTable` array-based specialization `typedef std::array<value_type, numeric_limits<unsigned_key_type>::max()> skip_map;` otherwise we could index out of the array (not an issue in the new one)
This comment above was marked as done but wasn't done AFAICT.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121074