martong added a comment.

> 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.


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

Reply via email to