[PATCH] D118038: [clang][dataflow] Enable merging distinct values in Environment::join

[Stanislav Gatev via Phabricator via cfe-commits]

sgatev added a comment.

Right. To model the behavior of a language feature (e.g. a simple model that tracks whether a `std::optional` value is engaged) clients can attach properties to symbolic values and decide how distinct values are joined. This doesn't completely rule out the possibility of running multiple clients in the same fixed-point iteration. In fact, we think this could be beneficial as clients can use each others' results by enriching the environment (e.g. dead code analysis can take advantage of understanding of whether a `std::optional` value is always (not) engaged). This is a direction that we're currently exploring. It does require some care to ensure that the clients are not stepping on each others' toes. For example, we should probably replace the current `Properties` mechanism with something more robust to ensure that properties are stored in isolated namespaces.



================
Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h:67
+    ///  `Val1` and `Val2` must be distinct.
+    virtual void merge(QualType Type, const Value &Val1, const Value &Val2,
+                       Value &MergedVal, Environment &Env) {}
----------------
xazax.hun wrote:
> Previously, when the values were distinct, we did not include anything in the merged environment. With the new model, we will end up creating "default" values for every one of them. I wonder if this is wasteful. We could potentially also defer this until we have some real world data and can benchmark this. But I think we could consider changing the return type to bool to specify if the merged value should be included in the resulting environment at all and this could return false by default.
Good idea. Let's only include the merge value if necessary for now. I changed the interface to return a bool indicating whether the merged value should be included or not. I added a FIXME to consider not storing the constructed value in the context if it isn't necessary. I'd prefer to address that separately, to keep the scope of this patch smaller.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D118038/new/

https://reviews.llvm.org/D118038