Summary:
tbv2: remove unnecessary copy in Element
`IntrospectionResult::const_iterator` iterates through the `Element`s in an
`IntrospectionResult`. `Element` currently copies the `type_path` which is a
`std::vector<string_view>` every time the iterator is incremented. This is
unnecessary as the data in the vector only changes slightly between iterations.
This change changes the `type_path` field in `Element` to a
`std::span<const std::string_view>`. Doing this previously caused SEGVs because
of the iterator's potential to be copied. To make it possible we do two things:
1. Make all copies explicit using a clone interface as in `ContainerInfo`. This
prevents accidental copies of an expensive structure.
2. After calling the copy constructor in `clone()` update the `span` in `next_`
to point at the newly copied structure.
Moves are fine because the `span` points at the allocation of the `vector`, not
the vector itself.
Test Plan:
- CI
- `FILTER='OilIntegration.*' make test`
- Ran `OilgenIntegration.std_vector_vector_int_some` which SEGVd with the
`span` change before and now doesn't. This now passes cleanly with ASAN
enabled on the target, though isn't available in `main` (only works on my
machine).
Differential Revision: D53472595
Pulled By: JakeHillion
C++ has a concept of Primitive which holds in the type graph. However we don't
currently expose this information to the end user. Expose this from the OIL
iterator to allow future features like primitive rollups.
This affects containers like maps which have a fake `[]` element with no type.
They use this to group together the key/value in a map and to account for any
per element storage overhead. Currently the decision is to make the fake `[]`
element a primitive if all of its children are primitives. This allows for more
effective primitive rollups if that is implemented. This implementation detail
may be changed in future.
Test Plan:
- CI
- Updated simple tests.
Summary:
Previously on large types OIL would have problems with corrupting the `std::stack<exporter::inst::Inst>` that is passed to the processors. This change hides the implementation of the stack from the processors by wrapping the call to emplace in a `std::function` written by the non-generated code, which solves the test case I've seen for this crashing. It also allows us to easily change the stack implementation in future - I plan to change it to a `std::stack<T, std::vector<T>>` in a follow up.
Reviewed By: tyroguru
Differential Revision: D49273116