[PATCH] D141625: [DeclContext] Sort the Decls before adding into DeclContext

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D141625#4066362 <https://reviews.llvm.org/D141625#4066362>, @steven_wu wrote:

> @dblaikie Do you have any objection for committing the patch as it is? I don't think reverse iteration test is a proper way to test for this specific bug.

I think reverse iteration is the right way to test this specific bug (& any bug where behavior may be unintentionally nondeterministic due to non-deterministically ordered containers) - it makes the testing more reliable rather than depending on implementation details of such containers that aren't guaranteed (that being the point/problem).

But it's not the worst/most unwieldy test for this sort of thing & if we don't have bots using it... hmm, actually I looked more closely & maybe we do have reverse iteration builders: https://github.com/llvm/llvm-zorg/search?q=reverse-iteration (though I'm having trouble navigating the builder UI to see whether there are up-to-date builds for this configuration)

Could you help me understand your perspective regarding using reverse iteration to test this specific bug? (are there some bugs you find reverse iteration suitable for? what's different about this one from them?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141625