[PATCH] D116596: [clang][dataflow] Add transfer functions for assignment

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added inline comments.


================
Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h:34
+/// storage locations or values.
+enum class SkipPast {
+  /// No indirections should be skipped past.
----------------
I am just wondering if this is the right level of abstraction. Maybe users do think of this in terms of skipping references? 
Or would they think in terms of value categories? E.g., having `getLValue` vs `getRValue`. I think we can leave this as is for now, I just like the idea of exploring alternatives.


================
Comment at: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:95
+StorageLocation &Environment::createStorageLocation(const Expr &E) {
+  // Evaluated expressions are always assigned the same storage locations to
+  // ensure that the environment stabilizes across loop iterations. Storage
----------------
What is the model for expressions that evaluate to different locations in every loop iterations, e.g.:
```
for(int *p = array; p != array + size; ++p)
  *p; // The dereference expression
```

Here, having `*p` always evaluate to the same location is not correct, we'd probably need a way to widen this. 


================
Comment at: clang/lib/Analysis/FlowSensitive/Transfer.cpp:46
+
+      Env.setValue(*LHSLoc, *RHSVal);
+    }
----------------
I think you probably also want to set the correct location for the whole expression. Or is that handled somewhere else?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116596