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

Thu Feb 2 12:47:06 PST 2023

dblaikie added a comment.

In D141625#4100660 <https://reviews.llvm.org/D141625#4100660>, @akyrtzi wrote:

>> would be great to test it more in a semantic way if possible
>
> Keep in mind that the specific order of the decls doesn't matter for the purposes of this test, what matters is that the order is the same every time for the same input.
>
> I personally think that the addition of "Spot check entries to make sure they are in current ordering" is counter-productive, because if later on some clang changes end up changing the order then the test will fail, and will need update, but that it is not what the test should care about, it should only fail if the order is non-deterministic.
> But I don't feel strongly about it, I'm fine with adding the ordered check even though I don't think it's a good idea.

I agree it's brittle/not ideal/a tradeoff - I'd like a test that is more stable to /some/ unrelated changes (ie: not testing the numbered values, but testing something a bit more semantically).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141625