Skip to content

Almost finalize main api of cetl::any - now close to std::any features. - #104

Merged
serges147 merged 4 commits into
issue/83_anyfrom
sshirokov/83_any_api
Mar 20, 2024
Merged

serges147 merged 4 commits into
issue/83_anyfrom
sshirokov/83_any_api

Conversation

@serges147

Copy link
Copy Markdown
Contributor
  • Added "value return" overloads of any_cast (#1️⃣, 2️⃣ & 3️⃣).
  • Added std::initializer_list overloads for
    • ctor (#6️⃣)
    • emplace (#2️⃣)
    • make_any (#2️⃣)
  • Added corresponding unit tests, plus more cases to increase coverage.

Notes for to be continued...

  • Align param fixes will be in the next pr.
  • Copyable/Movable (as well as elimination of various const_cast) will be in the after next pr.

…tures.

- Added "value return" overloads of any_cast (#1️⃣, 2️⃣ & 3️⃣).
- Added std::initializer_list overloads for
  - ctor (#6️⃣)
  - `emplace` (#2️⃣)
  - `make_any` (#2️⃣)
- Added corresponding unit tests, plus more cases to increase coverage.
@serges147 serges147 self-assigned this Mar 20, 2024

@pavel-kirienko pavel-kirienko left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

soo_handler::create needs this

assert(nullptr == dest.handler_);

before assigning dest.handler_ = handle;

Comment thread cetlvast/suites/unittest/test_any.cpp Outdated
Comment thread include/cetl/any.hpp
///
/// Equivalent to `cetl::any(cetl::in_place_type<ValueType>, list, std::forward<Args>(args)...)`.
///
template <typename ValueType, typename Any, typename Up, typename... Args>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we could default Any to something like this:

Suggested change
template <typename ValueType, typename Any, typename Up, typename... Args>
template <T>
using any_like = any<sizeof(T),
std::is_copy_constructible<T>::value,
std::is_move_constructible<T>::value,
std::max(alignof(T), alignof(std::max_align_t))>;
template <typename ValueType, typename Any = any_like<ValueType>, typename Up, typename... Args>

I only thought about this now but it should have been during the previous review.

Do you think it makes sense? Otherwise we deviate from the semantics of std::make_any quite a bit as it doesn't require one to specify the type of any.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it does. I even thought about it but didn't come up with your quite simple solution. Let me please add this topic to my "todo list" for some next PR. Thanks!

Comment thread cetlvast/suites/unittest/test_any.cpp Outdated
Comment thread cetlvast/suites/unittest/test_any.cpp Outdated
Comment thread include/cetl/any.hpp Outdated
Comment thread cetlvast/suites/unittest/test_any.cpp

@pavel-kirienko pavel-kirienko left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

assert is still missing, please see the previous review comment

@sonarqubecloud

Copy link
Copy Markdown

@serges147
serges147 merged commit 9d4bcf8 into issue/83_any Mar 20, 2024
@serges147
serges147 deleted the sshirokov/83_any_api branch March 20, 2024 15:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants