balazske added a comment. In D66538#1641310 <https://reviews.llvm.org/D66538#1641310>, @martong wrote:
> > It looks like that the original code is correct in the decision of > > structural equivalence of the original pair. If we have an (A,B) and (A,C) > > to compare, B and C are in different decl chain, then (A,B) or (A,C) will > > be non-equivalent (unless B and C are equivalent, but what to do in this > > case?). The problem was that the code assumed that in this case always A > > and B (or A and C) are non-equivalent. If NonEquivalentDecls is not filled > > in this case (or not used at all) the problem does not exist. > > I think we must not update the NonEquivalentDecls cache at that level, > because as we've seen (during the discussion of > https://reviews.llvm.org/D62131) it may store the wrong pairs. > However, it is still okay to cache the inequivalent decls in one level > upper, right after the `Finish` returns. > See this diff > https://github.com/martong/clang/compare/NonEqDecls_cache_in_an_upper_level~...martong:NonEqDecls_cache_in_an_upper_level?expand=1 > Right now I started a build on our CI to see if it causes any slowdown. This is correct, `NonEquivalentDecls` can be filled after `Finish` (in `IsEquivalent`?) (still we can have similar cache for equivalence too, maybe this is not of so much use). With the new code there is the possibility to get more information about non-equivalence, the `NonEquivalentDecls` can be filled in `Finish` too like it is done now, and with not much effort (I think) we can add some dependency relation (a tree structure) to decide which (already visited) Decls become non-equivalent if one non-equivalence is found. If there is a `void f(A *, B *)` to check the `f` can be a "parent" of `A` and `B`, if `A` or `B` is found to be non-equivalent we can set this for `f` too. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66538/new/ https://reviews.llvm.org/D66538 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
