The shape of a sharing map is now different than before, as now leafs can be
directly attached to internal nodes. This adapts the various unit tests that
check that the map has a certain shape.
Previously the leaf nodes were a separate type, and internal nodes and container
nodes were represented by the same type. This changes the representation such
that all nodes are represented by sharing_nodet. This will allow an internal
node to directly point to a leaf node (instead of having to point to a container
node which in turn points to a leaf node).
This reduces the size of an irept by 5 pointers (i.e., 40 bytes on
64-bit systems). On SV-COMP's ReachSafety-ECA with this change we can
perform 3819.81 symex_step calls per second, compared to 2752.28 calls
per second with the std::map configuration. The performance improvements
are spread across various `irept`-related operations.
This replaces small_shared_two_way_ptrt with small_shared_n_way_ptrt. The new
shared pointer type allows more than two types for the managed objects. This can
be useful e.g. for implementing graph data structures with sharing where there
are more than two different node types.
1) pointer_object((T1 *)NULL) equals pointer_object((T2 *)NULL) for any
types T1, T2. Previously, this would return false, unless T1==T2.
2) Do not restrict the above to NULL, but instead let the existing logic
in simplify_inequality fully simplify this.
3) Add a unit test of this code, which highlighted further bugs and
limitations: the unit test previously did not set the instance of the
desired dynamic object, and address-of inequalities over dynamic objects
can also be simplified.
On some SV-COMP benchmark categories, hashing accounts for >20% of CPU
time (with profiling enabled) - top five:
* ReachSafety-BitVectors: 29.29% (470.54 seconds, which reduces to 4.39
seconds; for benchmarks not timing out we save 170 seconds (25%) in
non-profiling mode)
* Systems_BusyBox_NoOverflows: 27.98% (284.15 seconds, which reduces to
1.74 seconds; for the 1 benchmark not timing out we save 23 seconds
(6%) in non-profiling mode)
* Systems_BusyBox_MemSafety: 24.24% (194.74 seconds, which reduces to
0.93 seconds; no measurable difference on the 2 benchmarks not
failing/timing out)
* NoOverflows-Other: 18.84% (1127.61 seconds, which reduces to
23.57 seconds; for benchmarks not timing out we save 5 seconds (7%) in
non-profiling mode)
* ReachSafety-ControlFlow: 17.75% (1194.04 seconds, which reduces to
29.17 seconds; for benchmarks not timing out we save 200 seconds (25%)
in non-profiling mode)
For ReachSafety-ECA it's only 4.7%, which still amounts to 3006.7
seconds. With this change this reduces to 323.07 seconds. On
ReachSafety-ECA, this enables 3055.35 symex_step calls per second over
2752.28 calls per second without this change.
This adds sharing map unit tests to check that operations fail as expected. For
example, calling map.replace(key, value) when the key does not exist in the map
should fail.
This adds unit tests to check that the references into the sharing map in the
views and delta views remain valid after operations erase(), insert(), and
replace(). The references should remain valid to those elements that are not
changed by the respective operations.
This permits an in-place update, avoiding needless copy-out / mutate / move-in cycles for
expensive-to-copy value types without leaking a non-const reference to a value.
These were accidentally disabled when distinguishing ID_is_dynamic_object (a predicate that tests
whether an object is dynamic) from ID_dynamic_object (a reference to the object itself, similar to
symbol_exprt). I also take the opportunity to restore pretty-printing of dynamic object expressions
(while also keeping pretty-printing of the predicate).
Previously when sharing_map.erase(key) was called, two traversals of the path to
the leaf to erase were done. One to check whether the key was in the map, and if
it was, a second one to copy and detach the nodes on the path to the leaf to
erase. This commit changes erase() to require that the given key exists in the
map. This simplifies the implementation and avoids two traversals of the path to
the leaf to erase when it is known that the key exists. If it is not known
whether the key exists, sharing_map.has_key(key) should be explicitely called
first.
The data member and the write_* methods of sharing_node_innert and
sharing_node_leaft are made protected and existing external callers are
refactored to not use write_* directly.
This adds a reset() method which clears the contents of the shared pointer.
Furthermore, the code to remove a reference to the pointed-to object is factored
out into a method destruct(). The method is used both by the destructor and by
reset().
This is follow-up from a discussion on PR #3998, and a comment by
@tautschnig.
This function always fails, with an exception, when given anything but a
constant_exprt.
This change means that the caller must do the type conversion. The benefit
is to make the caller more aware of the requirement that this must be a
constant, and to make the caller handle the error appropriately (with an
user-friendly error message) in case this is not possible.
The disadvantage is additional code at the call site.
These are intended as helpers for when you want to convert a string to
and integer, but don't want to assert the result (e.g. when dealing with
user input there's usually something better you can do than outright
crashing if the conversion fails), but also don't want to deal with
exceptions (which involve more code to write and read and it's easy to
handle the wrong set of exceptions, whether it is too many or too few).
This commit adds templates for `expr_try_dynamic_cast` and
`type_try_dynamic_cast` where the parameter is an rvalue and the return
type is an `optionalt`. This is implemented by moving the parameter into
the `optionalt`.
Included are unit tests of the new templates, which show that they
return the types and values expected. As well as tests and a static
assert for the existing overloads which show that they still return a
pointer.
These new templates are useful in the case where we are using the
result of a function, which returns by value but only in the case where
the value can be cast to a given type. For example with the new
overloads the following code can be written -
```
exprt enig();
void handle_struct_case(const struct_exprt &struct_expr);
void myFunction()
{
if(const auto my_struct = expr_try_dynamic_cast<struct_exprt>(enig()))
handle_struct_case(*my_struct);
}
```
However without the new templates and because the old ones do not bind
to rvalues, an additional temporary variable otherwise would have to be
declared -
```
void myFunction2()
{
const exprt enigma = enig();
if(const auto my_struct = expr_try_dynamic_cast<struct_exprt>(enigma))
handle_struct_case(*my_struct);
}
```
Previously we stored a reference to the name_prefix parameter in
allocate objects that led to segfaults if it was constructed with
a temporary. Now we store a copy instead, which prevents that from
happening.
The end result of this commit is that code like the following example
can be used to construct `json_arrayt` / `json_objectt` -
```
const std::vector<std::string> input{"foo", "bar"};
const auto json_array =
make_range(input)
.map(constructor_of<json_stringt>())
.collect<json_arrayt>();
```
This commit includes -
* Constructors from iterators for json_arrayt and json_objectt, so
that the iterators from a range can be used to construct these classes.
* A `collect` member function for `ranget`, so that a chain of range
operations can finish with the construction of a resulting container
containing a collection of the results.
* A `constructor_of` template function, which provides syntactic sugar
when using `map` to call a constructor, compared to writing a new lambda
function each time such an operation is carried out.
* Unit tests covering all of the above functionality.
Michael Tautschnig