amccarth added a comment.

Wow, cool bug.

It's too bad the original code re-used an iterator variable instead of make a 
new name (which would have helped the compiler find the problem).  Note that 
the one they used is shadowed just a couple lines later.

It's too bad the original code feels it's necessary to create iter and end_iter 
up front.  I know raw loops require this trick to avoid re-computing the end 
iterator on each iteration through the loop, but that shouldn't be necessary on 
algorithms like `find_if`.

It's too bad that erase with an end iterator isn't just a safe no-op, so that 
zillions of callers aren't required to check find's return value.  Without the 
visual noise, it would be easier to write exactly what you want.

It's too bad the compiler cannot recognize that `find_if` has no side effects 
and thus ignoring its return value makes the statement a no-op.

It's too bad the `std::erase(std::remove_if(...))` idiom is so cumbersome.  I 
realize that would likely be overkill here, since you apparently want to erase 
just the first one that matches the predicate.  Nonetheless, it would be harder 
to make this kind of bug.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74010



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to