dblaikie added a comment. In D141625#4095197 <https://reviews.llvm.org/D141625#4095197>, @steven_wu wrote:
>> Sorry, I'm still not really following - OK, sounds like you're saying this >> test does fail at HEAD/without this patch in reverse iteration mode, and is >> a bit larger than would be minimally necessary to achieve that, but also >> might fail at HEAD without reverse iteration, providing somewhat more >> testing than if it were fully minimized/only caught in reverse. >> >> Fair enough -I don't think it's the right tradeoff, but I'm glad it's >> stable/provides that coverage. > > The main motivations are: > > - Reverse iteration coverage from bots are really low. > - The FileCheck that supposed to fail on reverse iteration is not fully > checking the order of decls in the serialized bitcode in a semantic way. It > is only checking an index, which also assigned based on an iteration order. > If both iterations are iterating none deterministic container, both will be > reversed and the test will pass :) That seems like unfortunately brittle testing - would be great to test it more in a semantic way if possible. (& then it'd also be more capable of testing more robustly in reverse iteration, I guess?) 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
