Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use CETL's polymorphic RTTI #110

Merged
merged 11 commits into from Apr 1, 2024
Merged

Use CETL's polymorphic RTTI #110

merged 11 commits into from Apr 1, 2024

Conversation

serges147
Copy link
Contributor

@serges147 serges147 commented Mar 27, 2024

  • Now CETL's polymorphic RTTI casting is in use at cetl::detail::base_storage.
  • Extended unit tests TestXxx helper types with virtual what() method - to test polymorphic RTTI casting.
  • Added compile tests to verify several static_assert-s at cetl::any (related to Footprint fitting).

Also:

  • Reduced code duplication at unit tests TestXxx helper types.
  • Fixed incorrect applying of -fno-exceptions to C flags - should be C++ flags.
  • Addressed several unit tests todo-s. No todo-s anymore at production cetl::any code.
  • Added verification that there will be no value moved out of source any in case of bad cast attempt.

This change was also made, but decided to revert (for now) and save it as patch (for future reference):

Sergei Shirokov added 3 commits March 27, 2024 11:57
commit 95eeef842e36077e5e97d6e6764fe994dc3fdc30
Merge: d595551 e240e82
Author: Sergei Shirokov <sergei@Sergeis-Mac-mini.local>
Date:   Wed Mar 27 11:48:53 2024 +0200

    Merge branch 'sshirokov/83_any' into sshirokov/83_any_rtti

commit d595551d5f5506d2c067367ab57dd4d1fde741bb
Author: Sergei Shirokov <sergei@Sergeis-Mac-mini.local>
Date:   Tue Mar 26 17:48:31 2024 +0200

    implemented dynamic casting using CETL rtti

commit 1a435dfc788e7c8cd3a819cd6d70e7954ef5d919
Author: Sergei Shirokov <sergei@Sergeis-Mac-mini.local>
Date:   Tue Mar 26 11:45:32 2024 +0200

    less code duplication at test entities

commit 939d9d69565467fa5b8fa29dfd26d0f03b27c22f
Author: Sergei Shirokov <sergei@Sergeis-Mac-mini.local>
Date:   Tue Mar 26 10:41:20 2024 +0200

    add `const_value_caster_`

commit fd8557712ff7558ea15bcc4fb7593a90cb36542e
Merge: 9ce28d4 b6c6bd5
Author: Sergei Shirokov <sergei@Sergeis-Mac-mini.local>
Date:   Tue Mar 26 09:31:24 2024 +0200

    Merge branch 'sshirokov/83_any' into sshirokov_83_any_rtti

commit b6c6bd5
Author: Sergei Shirokov <sergei@Sergeis-Mac-mini.local>
Date:   Tue Mar 26 09:16:00 2024 +0200

    Added intermediate `base_handlers` type (between `base_storage` and `base_copy`).

commit 568639c
Author: Pavel Kirienko <pavel.kirienko@gmail.com>
Date:   Mon Mar 25 22:12:01 2024 +0200

    Fix CI tests (incorrectly instantiated and initialized any)

commit c0f6beb
Author: Sergei Shirokov <sergei@Sergeis-Mac-mini.local>
Date:   Mon Mar 25 16:38:02 2024 +0200

    clang format

commit fb88cf3
Merge: 1e6e5db 6a315ce
Author: Sergei Shirokov <sergei@Sergeis-Mac-mini.local>
Date:   Mon Mar 25 16:34:15 2024 +0200

    Merge branch 'sshirokov/83_any' of https://github.com/OpenCyphal/CETL into sshirokov/83_any

commit 1e6e5db
Author: Sergei Shirokov <sergei@Sergeis-Mac-mini.local>
Date:   Mon Mar 25 16:34:08 2024 +0200

    PR fixes

commit 6a315ce
Author: Pavel Kirienko <pavel.kirienko@gmail.com>
Date:   Mon Mar 25 10:58:04 2024 +0200

    Trivial adjustments

commit 9ce28d433a549e70323744e0b60c5674018b48d3
Author: Sergei Shirokov <sergei@Sergeis-Mac-mini.local>
Date:   Mon Mar 25 09:18:05 2024 +0200

    Implemented `const std::type_info& any::type() const noexcept` is it's at `std::any`.

commit 8d150fa81e42cfd8fe3c53840d8991a1d743e116
Author: Sergei Shirokov <sergei@Sergeis-Mac-mini.local>
Date:   Mon Mar 25 09:12:12 2024 +0200

    Added new `CETLVAST_DISABLE_CPP_RTTI` configuration parameter of CETL.

    Allows to test compilation (and running unit tests) with and without c++ RTTI support.

commit f3aa4ed
Author: Sergei Shirokov <sergei@Sergeis-Mac-mini.local>
Date:   Fri Mar 22 16:11:49 2024 +0200

    try to fix CI by temporary comment out problematic expectations.

commit 4956e43
Author: Sergei Shirokov <sergei@Sergeis-Mac-mini.local>
Date:   Fri Mar 22 15:32:55 2024 +0200

    revert of previous: try to fix CI by temporary comment out problematic expectations.

    `std::atomic<bool> moved_{false};` is in use at tests to tackle compiler optimizations.

commit 101e941
Author: Sergei Shirokov <sergei@Sergeis-Mac-mini.local>
Date:   Fri Mar 22 14:22:45 2024 +0200

    try to fix CI by temporary comment out problematic expectations.

commit 531557a
Author: Sergei Shirokov <sergei@Sergeis-Mac-mini.local>
Date:   Fri Mar 22 14:06:32 2024 +0200

    try to fix CI by temporary comment out problematic expectation.

commit 86dd7fa
Author: Sergei Shirokov <sergei@Sergeis-Mac-mini.local>
Date:   Fri Mar 22 13:53:07 2024 +0200

    try to fix CI by temporary comment out problematic expectation.

commit 0d06033
Author: Sergei Shirokov <sergei@Sergeis-Mac-mini.local>
Date:   Fri Mar 22 11:50:54 2024 +0200

    more `moved_` unit tests fix

commit 5014cd2
Author: Sergei Shirokov <sergei@Sergeis-Mac-mini.local>
Date:   Fri Mar 22 11:48:44 2024 +0200

    unit tests fix

commit 7392872
Author: Sergei Shirokov <sergei@Sergeis-Mac-mini.local>
Date:   Fri Mar 22 11:37:00 2024 +0200

    better coverage of `swap(any&&)`

commit 174e53f
Author: Sergei Shirokov <sergei@Sergeis-Mac-mini.local>
Date:   Fri Mar 22 10:33:20 2024 +0200

    simplify `reset` logic

commit c65fc06
Author: Sergei Shirokov <sergei@Sergeis-Mac-mini.local>
Date:   Fri Mar 22 10:31:31 2024 +0200

    Reworked handling of get, copy, move and destroy functionality.

    - Now `Copyable` and `Movable` constraints are respected.
    - Now instead of single handler we have 3 (destroyer, copier, mover) - allows to eliminate AUTOSAR `const_cast` violations.
    - In unit tests:
      - more tests to cover combinations of `any` template params
      - switch from pointer based `any_cast` to value or reference based.
@serges147 serges147 self-assigned this Mar 27, 2024
@serges147 serges147 marked this pull request as ready for review March 27, 2024 15:34

value_type_informer_ = [](const void*) {

#if __cpp_rtti
Copy link
Member

Choose a reason for hiding this comment

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

Here, we are stepping on a slippery slope of relying on implementation-specific features. In general, as far as I know, the C++ standard provides no feature test macro for RTTI:

image

I would limit our implementation of any to CETL RTTI only.

From our earlier conversations, I understood that you were adding the type informer handler value_type_informer_ in order to support CETL RTTI rather than the standard typeid.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, I didn't know that __cpp_rtti is some kind of an extension. I will remove related stuff and stick with our CETL RTTI only. Could you please give a link (or pdf) from where you made the screenshot - for future reference. Thnx!

Copy link
Member

Choose a reason for hiding this comment

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

include/cetl/any.hpp Outdated Show resolved Hide resolved
Copy link

sonarcloud bot commented Mar 28, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
36.3% Coverage on New Code (required ≥ 90%)

See analysis details on SonarCloud

@pavel-kirienko
Copy link
Member

I think it's fine overall but there's one case that should be working but it's not; there's a missing remove_cv somewhere:

#include <cetl/any.hpp>
#include <iostream>
#include <string>


namespace cetl
{
template <>
constexpr type_id type_id_value<int> = {1,2,3,4,5};
template <>
constexpr type_id type_id_value<std::string> = {1,2,3,4,6};
}


int main()
{
    cetl::any<64> any;
    any.emplace<std::string>("hello");
    std::cout << *cetl::any_cast<std::string>(&any) << std::endl;
    std::cout << *cetl::any_cast<const std::string>(&any) << std::endl;  // here
}

@serges147
Copy link
Contributor Author

>should be working but it's not

I'll take a look at this

@serges147 serges147 merged commit 3f1258a into issue/83_any Apr 1, 2024
31 of 32 checks passed
@serges147 serges147 deleted the sshirokov/83_any branch April 1, 2024 05:51
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.

None yet

3 participants