[libcxx-commits] [PATCH] D62451: Regex backreference [1/3] Fixes backreferences for extended grammar.

[Marshall Clow via Phabricator via libcxx-commits]

mclow.lists added inline comments.


================
Comment at: libcxx/test/std/re/re.alg/re.alg.search/extended.pass.cpp:506
+        assert(m.suffix().second == m[0].second);
+        assert(m.length(0) >= 0 && static_cast<size_t>(m.length(0)) == std::char_traits<char>::length(s));
+        assert(m.position(0) == 0);
----------------
Mordante wrote:
> mclow.lists wrote:
> > Why two checks for `m.length(0)` here?
> > 
> > Why not just:
> > ```
> > assert(static_cast<size_t>(m.length(0)) == std::char_traits<char>::length(s));
> > ```
> > since you know that `length(s) > 0`
> > (Obviously, this applies to several places)
> I agree your proposed style would also work. I emulated the other tests in this file, they all use this assert.
> It feels wrong to me to change the new tests to use your proposed style
> `assert(static_cast<size_t>(m.length(0)) == std::char_traits<char>::length(s));`
> This would make the style of the new test inconsistent with the style of the existing tests.
> Do you prefer to keep the new tests as I wrote them or shall I create a separate patch to modify the style of the existing test?
Just keep it the same as the others, and I'll clean them all up sometime.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62451