This patch adds a new interface for the join operation, now properly
called `join`. Originally, the framework offered a single `merge`
operation, which could serve either as a join or a widening. In
practice, though we found this conflation didn't work for non-trivial
anlyses, and split of the widening operation (`widen`). This change
completes the transition by introducing a proper `join` with strict join
semantics.
In the process, it drops an odd (and often misused) aspect of `merge`
wherein callees could implictly instruct the framework to drop the
current entry by returning `false`. This features was never used
correctly in analyses and doesn't belong in a join operation, so it is
omitted.
---------
Co-authored-by: Dmitri Gribenko <gribozavr@gmail.com>
Co-authored-by: martinboehme <mboehme@google.com>
In particular, it's important that we create the "fallback" atomic at
this point
(which we produce if the transfer function didn't produce a value for
the
expression) so that it is placed in the correct environment.
Previously, we processed the terminator condition in the
`TerminatorVisitor`,
which put the fallback atomic in a copy of the environment that is
produced as
input for the _successor_ block, rather than the environment for the
block
containing the expression for which we produce the fallback atomic.
As a result, we produce different fallback atomics every time we process
the
successor block, and hence we don't have a consistent representation of
the
terminator condition in the flow condition.
This patch includes a test (authored by ymand@) that fails without the
fix.
This template function casts the result of `getValue()` or
`getStorageLocation()` to a given subclass of `Value` or
`StorageLocation` (using `cast_or_null`).
It's a common pattern to do something like this:
```cxx
auto *Val = cast_or_null<PointerValue>(Env.getValue(E));
```
This can now be expressed more concisely like this:
```cxx
auto *Val = Env.get<PointerValue>(E);
```
Instead of adding a new method `get()`, I had originally considered
simply adding a template parameter to `getValue()` and
`getStorageLocation()` (with a default argument of `Value` or
`StorageLocation`), but this results in an undesirable repetition at the
callsite, e.g. `getStorageLocation<RecordStorageLocation>(...)`. The
`Value` and `StorageLocation` in the method name adds nothing of value
when the template argument already contains this information, so it
seemed best to shorten the method name to simply `get()`.
For the time being, we're keeping the `Strict` versions around as deprecated synonyms so that clients can be migrated, but these synonyms will be removed soon.
Depends On D156673
Reviewed By: ymandel, xazax.hun
Differential Revision: https://reviews.llvm.org/D156674
In the [value categories RFC](https://discourse.llvm.org/t/70086), I proposed that the end state of the migration should be that `getValue()` should only be legal to call on prvalues.
As a stepping stone, to allow migrating off existing calls to `getValue()`, I proposed introducing `getValueStrict()`, which would already have the new semantics.
However, I've now reconsidered this. Any expression, whether prvalue or glvalue, has a value, so really there isn't any reason to forbid calling `getValue()` on glvalues. I'm therefore removing the deprecation from `getValue()` and transitioning existing `getValueStrict()` calls back to `getValue()`.
The other "strict" accessors are a different case. `setValueStrict()` should only be called on prvalues because glvalues need to have a storage location associated with them; it doesn't make sense to only set a value for them. And, of course, `getStorageLocationStrict()` and `setStorageLocationStrict()` should obviously only be called on glvalues because prvalues don't have storage locations.
Reviewed By: ymandel, xazax.hun
Differential Revision: https://reviews.llvm.org/D155921
This parameter was already a no-op, so removing it doesn't change behavior.
Reviewed By: ymandel
Differential Revision: https://reviews.llvm.org/D150137
This parameter was already a no-op, so removing it doesn't change behavior.
Depends On D149144
Reviewed By: ymandel, xazax.hun, gribozavr2
Differential Revision: https://reviews.llvm.org/D149151
The Annotations helper class does not have a gtest or gmock dependency, but because it's bundled with the rest of TestingSupport, it gets one. By splitting it out, a target can use it without being forced to use LLVM's copy of gtest.
Reviewed By: GMNGeoffrey, sammccall, gribozavr2
Differential Revision: https://reviews.llvm.org/D141175
This patch adds an initial implementation for sign analysis, with the
following lattice (T: top, N: negative, Z: zero, P: positive, B: bottom):
T
/ | \
N Z P
\ | /
B
The lattice is implemented with `BoolValue` properties attached to other
`Value`s.
Differential Revision: https://reviews.llvm.org/D136668