[libcxx-commits] [PATCH] D93557: [libc++] [P0879] constexpr std::nth_element, and rewrite its tests.

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone added inline comments.


================
Comment at: libcxx/include/algorithm:5281
+                __first = __i;
+                continue;  // i.e., goto __restart
             }
----------------
ldionne wrote:
> This comment is anchored to something that doesn't exist anymore, so I would remove it.
You mean you'd remove `// i.e., goto __restart`? I prefer to leave it, for the same reason we leave the existing comments like `// _VSTD::__nth_element<_Compare>(__i, __nth, __last, __comp)` — they show what we //would// put on that line if it weren't troublesome for mechanical reasons (don't want recursion, can't use goto). I left a comment above: `// __restart: -- this is the target of a "continue" below`

I'm very much not a fan of using `continue` for non-trivial control flow, anyway. But I'll take another look at this code and see if I can maybe figure out a way to eliminate the `continue`. For example, we could definitely just indent lines 5284–5362 one more level and stuff them into an `else` block.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93557