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: Delegate checks exact compatibility of signatures #1141

Merged
merged 3 commits into from
Jan 21, 2022

Conversation

paulgessinger
Copy link
Member

As described in #1140, signatures are not checked for exact compatibility by Delegate.

This PR introduces an additional check where it will try to assign a given callable to an expected function pointer type generated from the signature given to Delegate. This assignment only succeeds if the signatures match exactly. This is enforced by a static_assert.

Fixes #1140

/cc @Corentin-Allaire

@paulgessinger paulgessinger added the Bug Something isn't working label Jan 20, 2022
@paulgessinger paulgessinger added this to the next milestone Jan 20, 2022
@codecov
Copy link

codecov bot commented Jan 20, 2022

Codecov Report

Merging #1141 (47cba3e) into main (787658a) will decrease coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1141      +/-   ##
==========================================
- Coverage   48.17%   48.16%   -0.01%     
==========================================
  Files         341      341              
  Lines       17649    17646       -3     
  Branches     8327     8327              
==========================================
- Hits         8502     8499       -3     
  Misses       3372     3372              
  Partials     5775     5775              
Impacted Files Coverage Δ
Core/include/Acts/TrackFitting/KalmanFitter.hpp 38.09% <ø> (-0.37%) ⬇️
.../Acts/TrackFitting/detail/VoidKalmanComponents.hpp 25.00% <ø> (ø)
Core/include/Acts/Utilities/Delegate.hpp 80.95% <ø> (ø)
...de/Acts/TrackFinding/CombinatorialKalmanFilter.hpp 33.06% <0.00%> (-0.18%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 787658a...47cba3e. Read the comment docs.

Copy link
Contributor

@Corentin-Allaire Corentin-Allaire left a comment

Choose a reason for hiding this comment

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

Looks good to me ! It is great that we will be able avoid thing similar to issue #1140 in future

@kodiakhq kodiakhq bot merged commit b6695fd into acts-project:main Jan 21, 2022
@paulgessinger paulgessinger modified the milestones: next, v17.0.0 Mar 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug: Delegate accepts approximate signatures
2 participants