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

fix: change typeid check to dynamic_cast #1409

Merged
merged 3 commits into from
Aug 10, 2022

Conversation

asalzburger
Copy link
Contributor

This PR changes the way we check for compatibility in the WhiteBoard as sometimes the typeid check would fail, even if the cast would work.

The reason for the failure is not fully understood, but a dynamic_cast is ok in this case, as this is not a time-critical operation.

@asalzburger asalzburger added the Component - Examples Affects the Examples module label Aug 9, 2022
@asalzburger asalzburger added this to the next milestone Aug 9, 2022
@codecov
Copy link

codecov bot commented Aug 9, 2022

Codecov Report

Merging #1409 (f28022c) into main (968993e) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #1409   +/-   ##
=======================================
  Coverage   47.79%   47.79%           
=======================================
  Files         380      380           
  Lines       20173    20173           
  Branches     9387     9387           
=======================================
  Hits         9641     9641           
  Misses       4083     4083           
  Partials     6449     6449           

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

@benjaminhuth
Copy link
Member

benjaminhuth commented Aug 9, 2022

Just a question out of curiosity: Is there a reason why the WitheBoard does not e.g. use std::any (that would bring these typechecks)? I could imagine that would be more robust than a custom logic, but I have not thought deeply about that.

@paulgessinger
Copy link
Member

Just a question out of curiosity: Is there a reason why the WitheBoard does not e.g. use std::any (that would bring these typechecks)? I could imagine that would be more robust than a custom logic, but I have not thought deeply about that.

This was written back on C++14 (not by me 😄) so that's why.

I think switching to std::any would also be a good idea at some point.

@andiwand
Copy link
Contributor

andiwand commented Aug 9, 2022

I wonder if this is due to const or using base classes. but std::any would result in the same problem because it will not do any fancy casting right?

@benjaminhuth
Copy link
Member

I wonder if this is due to const or using base classes. but std::any would result in the same problem because it will not do any fancy casting right?

Hmm as far as I know std::any is completely typesafe... Or what do you mean by fancy casting?

@kodiakhq kodiakhq bot merged commit fdc6075 into acts-project:main Aug 10, 2022
@paulgessinger paulgessinger added the backport develop/v19.x Backport this PR to the v19.x series label Aug 10, 2022
acts-project-service pushed a commit that referenced this pull request Aug 10, 2022
This PR changes the way we check for compatibility in the `WhiteBoard` as sometimes the typeid check would fail, even if the cast would work.

The reason for the failure is not fully understood, but a `dynamic_cast` is ok in this case, as this is not a time-critical operation.

(cherry picked from commit fdc6075)
kodiakhq bot pushed a commit that referenced this pull request Aug 10, 2022
…19.x] (#1413)

Backport fdc6075 from #1409.
---
This PR changes the way we check for compatibility in the `WhiteBoard` as sometimes the typeid check would fail, even if the cast would work.

The reason for the failure is not fully understood, but a `dynamic_cast` is ok in this case, as this is not a time-critical operation.
@paulgessinger paulgessinger modified the milestones: next, v20.0.0 Aug 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport develop/v19.x Backport this PR to the v19.x series Component - Examples Affects the Examples module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants