[PATCH] D135964: [clang][dataflow] Add equivalence relation for `Value` type.

Fri Oct 14 11:24:56 PDT 2022

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