steven_wu added a comment. In D141625#4094837 <https://reviews.llvm.org/D141625#4094837>, @dblaikie wrote:
> In D141625#4094742 <https://reviews.llvm.org/D141625#4094742>, @steven_wu > wrote: > >> In D141625#4067225 <https://reviews.llvm.org/D141625#4067225>, @dblaikie >> wrote: >> >>> In D141625#4066961 <https://reviews.llvm.org/D141625#4066961>, @steven_wu >>> wrote: >>> >>>> No, reverse iteration will not break diff test for a small number of >>>> decls. Everything will be in reverse order so it is the same. >>> >>> Hmm, I'm not sure I'm following why that is - could you explain this in >>> more detail? The usual problem is that, say, we're outputting based on an >>> unstable order - even two items would be enough to cause a test of that to >>> fail in either forward or reverse iteration mode until the order is >>> stabilized. >>> >>> Is that not the case here? Is there some interaction between iteration >>> order that's part of the nondeterminism here? >> >> In order to make a test that will break before the change with reverse >> iteration, the test needs to check that the decls/records are serialized >> into the module in a pre deterministic order. It can't be just diff the >> output of two runs with a small input because small input will not overflow >> the smallptrset, thus the reverse iteration outputs from two runs will very >> likely to be identical, just in a different order from forward iteration. > > Sure, I think I'm with you there - but the current test checks that the > decls/records are serialized into the module in a pre-deterministic order, > right? So it doesn't seem like a reverse iteration-failing test would be more > involved/brittle/less robust than the test being added here? Yes, exactly, I added file check so that this test is going to fail for reverse iteration. What I also want is keep the size of the test case since it not really enormous so this test also has a good chance of failing without reverse iteration. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141625/new/ https://reviews.llvm.org/D141625 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
