-
Notifications
You must be signed in to change notification settings - Fork 180
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: some blocker reported sonar-cloud issues #4009
fix: some blocker reported sonar-cloud issues #4009
Conversation
WalkthroughA comprehensive update to exception safety across multiple files in the Acts library has been implemented. The changes primarily involve adding Changes
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
Core/include/Acts/EventData/VectorMultiTrajectory.hpp
(2 hunks)Core/include/Acts/Geometry/GeometryHierarchyMap.hpp
(1 hunks)Core/include/Acts/Seeding/BinnedGroupIterator.ipp
(1 hunks)Core/include/Acts/Utilities/Any.hpp
(1 hunks)Core/include/Acts/Utilities/MultiIndex.hpp
(1 hunks)Core/include/Acts/Utilities/Result.hpp
(2 hunks)Examples/Detectors/GenericDetector/include/ActsExamples/GenericDetector/ProtoLayerCreatorT.hpp
(4 hunks)Examples/Framework/include/ActsExamples/EventData/GeometryContainers.hpp
(2 hunks)Examples/Framework/include/ActsExamples/Utilities/Range.hpp
(1 hunks)Plugins/FpeMonitoring/include/Acts/Plugins/FpeMonitoring/FpeMonitor.hpp
(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- Core/include/Acts/Utilities/Any.hpp
- Examples/Detectors/GenericDetector/include/ActsExamples/GenericDetector/ProtoLayerCreatorT.hpp
🧰 Additional context used
📓 Learnings (1)
Core/include/Acts/EventData/VectorMultiTrajectory.hpp (1)
Learnt from: paulgessinger
PR: acts-project/acts#3907
File: Core/include/Acts/EventData/VectorMultiTrajectory.hpp:349-351
Timestamp: 2024-11-26T10:57:04.925Z
Learning: In `VectorMultiTrajectory` class within `Core/include/Acts/EventData/VectorMultiTrajectory.hpp`, the initialization of `m_meas` and `m_measCov` is ensured by the rest of the interface.
🔇 Additional comments (9)
Examples/Framework/include/ActsExamples/Utilities/Range.hpp (1)
30-33
: Approve these changes, I do!Wise addition of
noexcept
to move operations, this is. Non-throwing, the move operations are, as only iterator members they contain. Follow the path of exception safety, we must.Plugins/FpeMonitoring/include/Acts/Plugins/FpeMonitoring/FpeMonitor.hpp (1)
Line range hint
48-54
: Approve this change, I do!Correctly marked as
noexcept
, the move constructor is. Only smart pointer and primitive types it moves. Strong in the ways of exception safety, this code becomes.Core/include/Acts/Utilities/MultiIndex.hpp (1)
73-73
: Approve this change, I do!Correctly marked as
noexcept
, the move assignment is. Only primitive type it moves. In harmony with exception safety guidelines, this code now is.Examples/Framework/include/ActsExamples/EventData/GeometryContainers.hpp (1)
165-165
: Approve parameter renaming, I do! More clarity it brings!Changed from
module
tosensitive
, the parameter name has been. Matches better with its purpose, it does, as a sensitive surface identifier it represents. Updated properly, the documentation has been.Also applies to: 169-169, 179-181
Core/include/Acts/Geometry/GeometryHierarchyMap.hpp (1)
82-82
: Approve these changes, I do! Stronger exception guarantees they bring!Added
noexcept
to move operations, wisdom shows. Safer and more efficient, the code becomes. Follow modern C++ best practices, it does.Also applies to: 85-85
Core/include/Acts/Utilities/Result.hpp (2)
46-46
: Approve this change, I do! Better exception guarantees it provides!Added
noexcept
to move operations ofResult<T,E>
, wisdom shows. Safe these operations are, as only moving a variant member they do.Also applies to: 51-54
360-360
: Approve this change too, I do! Consistency it maintains!Added
noexcept
to move constructor ofResult<void,E>
, wisdom shows. Matches the changes inResult<T,E>
, it does. Safe the operation is, as only moving an optional member it does.Core/include/Acts/EventData/VectorMultiTrajectory.hpp (2)
195-195
: Approve this change, I do! Clearer guarantees it provides!Added
noexcept
to default constructor, wisdom shows. Cannot throw, a default constructor does.
391-392
: Approve this change, I do! Consistency with base class it maintains!Added
noexcept
to move constructor, wisdom shows. Safe the operation is, as it delegates to base class move constructor which noexcept already is.
|
Spillover from 2024 - some "blocker" labelled SonarCloud issues fixed.
--- END COMMIT MESSAGE ---
Any further description goes here, @-mentions are ok here!
feat
,fix
,refactor
,docs
,chore
andbuild
types.Summary by CodeRabbit
noexcept
specifications to various move constructors and move assignment operators across multiple classes to improve exception safety and performance.ProtoLayerCreatorT
class for clarity without altering functionality.selectModule
function for better representation of its purpose.