In D54381#1294619, @lebedev.ri wrote:

I think the usage of Set is correct.
We will, at most, fail to deduplicate one element (the one we just removed) out of N returned by rangeQuery(Q).
But if we don't do any deduplication at all, then we will add all the N elements. And that sounds exponential.

I could be missing something, but I don't understand why ToProcess needs to be a set-like container since we're erasing elements as we go (ie the erased elements won't be duplicate checked on next insertion). We skip any that have been previously processed in the inner loop too, which seems like it's doing the same work the set would be doing.

In D53263#1294477, @kristina wrote:

Huge apologies, it seems I can't get this to patch cleanly against my fork and therefore can't test it before committing, which is something I generally always do. I'll leave it to someone else. Again, huge apologies, hopefully you won't have to wait too long.

In D53263#1294383, @kristina wrote:

I can do it if you'd like, will be a moment though.

Thanks and much appreciated!

Thanks!
I don't have commit access to land this myself.

Ping.

Ping.

Add missing part of previous change to patch. Now all unit tests pass.

Fixed the added assert to do the right thing. Clang can now build with a debug Clang built from this patch, and all unittests I could find pass in that built version too.

Pinging this as it's been "Ready to Land" for a while and I want to check that that's not because I've forgotten to do something?

Thanks! I don't have write access to svn so I can't commit the patch myself, but assuming that I've read the docs correctly if anyone is able to take it I'd be grateful.