ymandel marked an inline comment as done. ymandel added a comment. In D135964#3858783 <https://reviews.llvm.org/D135964#3858783>, @xazax.hun wrote:
> In D135964#3858706 <https://reviews.llvm.org/D135964#3858706>, @ymandel wrote: > >> Reviewers: I'm inclined towards a method vs overloaded operator. Please let >> me know. > > I don't have a strong preference. But in case we come up with multiple kinds > of equalities (see my comment about the properties) methods might work better. Agreed. I switched to a free function. I think `operator==` just implies too much. ================ Comment at: clang/lib/Analysis/FlowSensitive/Value.cpp:37 + areEquivalentIndirectionValues(Val1, Val2)) && + Val1.Properties == Val2.Properties); +} ---------------- xazax.hun wrote: > I started to wonder, is it always the case that we want properties to be part > of the identity for values? Especially when we have multiple checks > collaborating in the same fixed-point iteration, the properties added by one > check might not be interesting for the others, and influencing what values > are considered equal is a way to "leak" this information between checks. > > Feel free to leave it as is for now, I just wanted to make sure we think > about this at some point in the future :) Good point. This only occurred to me once I was writing it as an `operator==`. But, we don't do this in the current code and I think its premature. I've taken it out and updated the comment to reflect. We will need to consider this for the future. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135964/new/ https://reviews.llvm.org/D135964 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
