steven_wu added a comment. In D141625#4056831 <https://reviews.llvm.org/D141625#4056831>, @steven_wu wrote:
> Actually, sorting in `numberAnonymousDeclsWithin` doesn't work for some > reasons. The reason for this doesn't work is `ASTWriter::WriteDeclContextLexicalBlock` also iterates on `DeclContext::decls()`, so there are at least two sorts needed in ASTWriter. I prefer the current implementation if there isn't any performance overhead since it makes the iterator on DeclContext to have stable order. In D141625#4059186 <https://reviews.llvm.org/D141625#4059186>, @dblaikie wrote: > In D141625#4053067 <https://reviews.llvm.org/D141625#4053067>, @steven_wu > wrote: > >> @akyrtzi has the good idea. It is really hard to control `Decl*` to get >> values >> to get an unstable iteration order from the small tests, going beyond 32 >> decls >> to get out of SmallPtrSet's small model is much consistent. >> >> Add test. > > Could you make a smaller test (probably just a couple of decls) that fails > with LLVM_ENABLE_REVERSE_ITERATION? Such a failure would be more > reliable/simpler to reproduce, probably? Sure but it is going to put a different requirement on the tests. Now ASTWriter only cares about a stable order for deterministic output so it is doing a diff on pcm. If changing to the reverse iterator test, this need to change to do FileCheck on a predetermined order. 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
