sgatev 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.
----------------
xazax.hun wrote:
> 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.
Absolutely! It makes sense to me to consider this (and other) alternatives so I
added a FIXME as a reminder.
================
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
----------------
xazax.hun wrote:
> 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.
In the current model, the storage location for `p` is stable and the value that
is assigned to it changes. So, in one iteration the value assigned to the
storage location for `p` is `val1` which is a `PointerValue` that points to
`loc1` and in another iteration the value assigned to the storage location for
`p` is `val2` which is a `PointerValue` that points to `loc2`. The storage
location for `*p` is also stable and the value that is assigned to it is a
`ReferenceValue` that points to either `loc1` or `loc2`. I implemented this and
added a test.
================
Comment at: clang/lib/Analysis/FlowSensitive/Transfer.cpp:46
+
+ Env.setValue(*LHSLoc, *RHSVal);
+ }
----------------
xazax.hun wrote:
> I think you probably also want to set the correct location for the whole
> expression. Or is that handled somewhere else?
You're right. I updated the code and added a test.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D116596/new/
https://reviews.llvm.org/D116596
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
