dblaikie added a comment.

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?


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

Reply via email to