Also first revision of any methods, functions and tests - without RTTI yet. - #102
Conversation
…eset` `has_value` methods. Also first revision of standalone `any_cast` functions (without yet RTTI).
`cetl::pf17` will have very close to `std::any` specialization.
Also: - Appended `is_in_place_type_v` and `is_in_place_index_v` constexpr-s. - Applied clang format to `utility.hpp` header file.
Also more todos.
|
|
||
| /// A polyfill for `std::bad_any_cast`. | ||
| /// This is only available if exceptions are enabled (`__cpp_exceptions` is defined). | ||
| class bad_any_cast : public std::bad_cast |
There was a problem hiding this comment.
It makes sense that this class is here, as it polyfills std::bad_any_cast, but I wonder if it's going to be a problem later on when we need to add a specialization of our cetl::any<> into this file, because that would require this header to depend on cetl/any.hpp, which creates a recursive dependency.
If this is a valid concern, we might be better off moving everything that is needed for cetl/any.hpp into its own header.
There was a problem hiding this comment.
Yes, I though about it, but decided to tackle this when I start working on the specialization. Is it fine to postpone?
| { | ||
| case detail::action::Get: | ||
|
|
||
| return get(const_cast<any&>(*self)); |
There was a problem hiding this comment.
const_cast violates AUTOSAR A5-2-3
There was a problem hiding this comment.
I will eliminate all these const_cast-s when reworking the handle thing (and implementing Copyable/Movable constraints). is it ok to postpone to near future pr?
| Copy, | ||
| Move, | ||
| Destroy | ||
| }; |
There was a problem hiding this comment.
This code is fine but really cool kids from the block use algebraic types for this these days, because it allows one to express more concepts through the type system:
struct action_get{};
struct action_copy{ any* other; }; // notice the `other`!
struct action_move{ any* other; }; // it is introduced only when needed
struct action_destroy{};
using action = variant<action_get, action_copy, action_move, action_destroy>;Then you use visit to handle action in a type-safe way without the need to introduce unused parameters.
Just saying.
There was a problem hiding this comment.
I'll think about it. Also, maybe when I start working on Copyable/Movable constraints then it's possible that enum and switch won't be applicable anymore...
| { | ||
| static_assert(sizeof(Tp) <= Footprint, "Enlarge the footprint"); | ||
|
|
||
| static void* handle(detail::action action, const any* self, any* other) |
There was a problem hiding this comment.
The way I read it, non-const function parameters that are not mutated violate AUTOSAR A7-1-1.
There was a problem hiding this comment.
see my answer for line 218 plz
|
|
||
| case detail::action::Copy: | ||
|
|
||
| copy(*other, *self); |
There was a problem hiding this comment.
I mentioned this yesterday in the evening but just for historical purposes I want to mention it here again that this code will break if Tp is non-copyable. Same goes for moving. This is why we need these telescoping bases here that select the copy/move behaviors depending on the template parameters.
There was a problem hiding this comment.
Yes, I'll work on that in the next PR (if it's ok with you)
| { | ||
| using uut = any<sizeof(int)>; | ||
|
|
||
| const uut srcAny{42}; |
There was a problem hiding this comment.
Applies in other places as well.
| const uut srcAny{42}; | |
| const uut src_any{42}; |
There was a problem hiding this comment.
@pavel-kirienko is it fine if I fix this in separate pr? it will be just "cosmetic" pr without functional changes.
There was a problem hiding this comment.
Why can't we fix this now?
There was a problem hiding this comment.
yes we can - I just though it will be easier review separately "cosmetics" ONLY changes
There was a problem hiding this comment.
I don't think it makes a lot of difference here
There was a problem hiding this comment.
ok, I'll do it now, thnx
|
| template <std::size_t Footprint, bool Copyable> | ||
| struct base_copy; | ||
|
|
||
| template <std::size_t Footprint, std::size_t Align = sizeof(std::max_align_t)> |
There was a problem hiding this comment.
The alignment should be also configurable via the list of template parameters to any:
template <std::size_t Footprint,
bool Copyable = true,
bool Movable = Copyable,
std::size_t Align = alignof(std::max_align_t)>I neglected to mentioned this explicitly earlier. Also note that alignof should be used instead of sizeof.
There was a problem hiding this comment.
Align configurable will be ready (and tested) after next pr (already ready in my separate private branch).
std::any).operator=-s.any_cast.make_anyoverloads.has_value,emplace,swap&reset.cetl::bad_any_castnow alias tocetl::pf17::bad_any_cast.Also:
is_in_place_type_vandis_in_place_index_vconstexpr-s.utility.hppheader file.Related to #83