chore: fix more sonralcloud issues #4017
Conversation
WalkthroughThe pull request introduces namespace qualifications and explicit constructors across multiple header files in the Acts library. These changes enhance type safety by preventing implicit conversions and clarifying type origins. The modifications span various classes and structs, primarily adding Changes
Suggested Labels
Suggested ReviewersPoem
Walkthrough (Yoda Style)Hmm, changes subtle but powerful, they are! Namespace clarity, we seek. Constructors explicit, they become. Move operations safe, they now stand. Prevent implicit conversions, we must! A path to cleaner code, this is. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
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 (
|
github-actions
bot
added
Component - Core
github-actions
bot
removed
Component - Examples
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
Core/include/Acts/EventData/GenericFreeTrackParameters.hpp (1)
133-133: Strong with the force, these changes are!Noexcept move operations, correctly added they are. Member types, support noexcept moves they do.
Hmm, future improvement sense I do. When C++20 arrives, [[no_unique_address]] for ParticleHypothesis member, use you should. TODO comment about this, already exists it does.
Also applies to: 137-138
Alignment/include/ActsAlignment/Kernel/detail/AlignmentEngine.hpp (1)
38-67: Clear namespace origins, now we see! Hmmmm.Qualified with Acts:: namespace, all types now are. Clarity and wisdom, this brings to the code. Prevent confusion in the future, it will.
Consider using namespace alias to reduce verbosity, you might:
namespace Acts::detail { using Matrix = Acts::ActsDynamicMatrix; using Vector = Acts::ActsDynamicVector; // ... rest of the code }Also applies to: 105-107, 172-185, 205-242, 278-282
Core/include/Acts/EventData/VectorMultiTrajectory.hpp (1)
572-575: Approve these changes, I do! Hmmmm.Made explicit the constructors are, preventing accidental conversions they do. A path to safer code, this is.
Consider similar treatment for other constructors in the codebase, you should. A consistent approach to type safety, it brings.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
Alignment/include/ActsAlignment/Kernel/detail/AlignmentEngine.hpp(7 hunks)Core/include/Acts/Detector/DetectorVolume.hpp(1 hunks)Core/include/Acts/Detector/IndexedRootVolumeFinderBuilder.hpp(1 hunks)Core/include/Acts/Detector/LayerStructureBuilder.hpp(1 hunks)Core/include/Acts/Detector/Portal.hpp(1 hunks)Core/include/Acts/EventData/GenericBoundTrackParameters.hpp(1 hunks)Core/include/Acts/EventData/GenericCurvilinearTrackParameters.hpp(1 hunks)Core/include/Acts/EventData/GenericFreeTrackParameters.hpp(1 hunks)Core/include/Acts/EventData/MultiTrajectory.hpp(1 hunks)Core/include/Acts/EventData/ProxyAccessor.hpp(1 hunks)Core/include/Acts/EventData/SourceLink.hpp(1 hunks)Core/include/Acts/EventData/TrackStateProxy.hpp(1 hunks)Core/include/Acts/EventData/VectorMultiTrajectory.hpp(1 hunks)Core/include/Acts/EventData/VectorTrackContainer.hpp(1 hunks)Core/include/Acts/EventData/detail/CorrectedTransformationFreeToBound.hpp(1 hunks)Tests/UnitTests/Core/EventData/MultiTrajectoryTests.cpp(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- Core/include/Acts/EventData/GenericBoundTrackParameters.hpp
⏰ Context from checks skipped due to timeout of 90000ms (14)
- GitHub Check: CI Bridge / lcg_105: [alma9, gcc13]
- GitHub Check: CI Bridge / linux_ubuntu_2204_clang
- GitHub Check: CI Bridge / linux_ubuntu_2204
- GitHub Check: CI Bridge / build_linux_ubuntu
- GitHub Check: CI Bridge / build_exatrkx
- GitHub Check: CI Bridge / build_exatrkx_cpu
- GitHub Check: CI Bridge / clang_tidy
- GitHub Check: merge-sentinel
- GitHub Check: missing_includes
- GitHub Check: linux_ubuntu
- GitHub Check: macos
- GitHub Check: linux_ubuntu_extra (ubuntu2204_clang, 20)
- GitHub Check: linux_ubuntu_extra (ubuntu2204, 20)
- GitHub Check: build_debug
🔇 Additional comments (12)
Core/include/Acts/EventData/SourceLink.hpp (1)
32-32: Wise changes these are, approve them I do!Added noexcept to move operations, improve exception safety guarantees it does. Strong exception guarantee, the force is with this code.
Also applies to: 34-34
Core/include/Acts/EventData/GenericCurvilinearTrackParameters.hpp (1)
94-95: Consistent with the force, these changes are!Noexcept move operations, aligned with base class they are. Strong exception safety, inherited from GenericBoundTrackParameters it is.
Also applies to: 100-100
Core/include/Acts/Detector/IndexedRootVolumeFinderBuilder.hpp (1)
29-30: Wise change, this is!Prevent accidental conversions from vector to IndexedRootVolumeFinderBuilder, the explicit keyword does. Strong in the ways of type safety, you have become.
Core/include/Acts/EventData/ProxyAccessor.hpp (1)
73-73: Hmmmm... Good changes these are!Prevent implicit conversions from HashedString and std::string, these modifications do. Safe from the dark side of type conversions, your code now is.
Also applies to: 77-77
Core/include/Acts/EventData/detail/CorrectedTransformationFreeToBound.hpp (1)
75-76: Strong with the Force, this change is!Prevent accidental transformations from FreeToBoundCorrection, the explicit keyword does. Clear intentions in your code, you now have.
Core/include/Acts/Detector/LayerStructureBuilder.hpp (1)
61-61: Mmmmm... A path to clarity, you have found!Prevent unintended surface vector conversions, this explicit constructor does. Protected from the dark side of implicit conversions, your SurfacesHolder now is.
Core/include/Acts/Detector/Portal.hpp (1)
50-50: Wise addition of explicit keyword, this is!Prevent accidental conversions from shared_ptr to Portal, this change does. A path to safer code, it is.
Tests/UnitTests/Core/EventData/MultiTrajectoryTests.cpp (1)
95-95: Changed initialization style, you have. Good, this is!From copy initialization to direct initialization, the path leads. With explicit constructors, this aligns better.
Core/include/Acts/EventData/VectorTrackContainer.hpp (1)
285-293: Strong with type safety, these changes are!Two constructors, explicit now they become. Prevent accidental conversions, they do, while maintaining both copy and move powers.
Core/include/Acts/Detector/DetectorVolume.hpp (1)
87-87: Wise decision this is, approve it I do!Explicit constructor prevents accidental conversions from std::vector to ObjectStore. Strong in the ways of type safety, this change is.
Core/include/Acts/EventData/MultiTrajectory.hpp (1)
111-111: Approve this change, I must!Made explicit the constructor is, preventing accidental conversions from ProxyType to TrackStateRange. In harmony with the Force of type safety, this change is.
Core/include/Acts/EventData/TrackStateProxy.hpp (1)
45-45: Strong with type safety, this change is. Approve it, I do!Made explicit the constructor is, preventing the dark side of implicit pointer conversions. A wise path to safer code, this follows.
|
Some more sonar cloud issue fixes from
Core/EventDatamainly.--- END COMMIT MESSAGE ---
Any further description goes here, @-mentions are ok here!
feat,fix,refactor,docs,choreandbuildtypes.Summary by CodeRabbit
Code Quality
explicitto various constructors.noexceptspecifiers to move constructors and assignment operators.Refactor