Kratos
Kratos copied to clipboard
[Core] Fixing const-correctness in kratos pointers
📝 Description
There are many const pointer that should be pointer to const. See https://github.com/KratosMultiphysics/Kratos/issues/9779 for a more detailed explanation.
🆕 Changelog Defined:
PointerToConst(shared)PointerToConst(intrusive)SharedPointerToConstWeakPointerToConstUniquePointerToConst
Fixed return type of "const pGetters" from const Type::Pointer to Type::PointerToConst.
Breaking changes
The breaking changes were fixed in the applications. Users of Kratos outside the repository may need to remove const from some elements, conditions and modelparts to acomodate these changes. To avoid any chaos, a deprecation warning will be added for some time before merging this change. See #9790.
Unfixed issue
Method Clone of elements and conditions should not be const, since it includes a call to the non-const version of pGetProperties.
Fixing this would be too much (it would break practically every single element and condition). To avoid doing this work, I decided too keep pGetProperties with a the wrong return type Properties::Pointer rather than Properties::PointerToConst.
This is left outside the scope of this PR.
@KratosMultiphysics/technical-committee has been discussing about this again.
We are in favour of the change as current behaviour might be misleading.
However, we're afraid of how this propagates through the code. In this regard, we would like to know the opinion of the @KratosMultiphysics/implementation-committee.
Speaking on my own (to be discussed in the @KratosMultiphysics/implementation-committee ), I agree that the current notation is confusing (I was not sure of this behaviour until now!) so I would vote in favour of changing it. Especially after reading the very insightful https://github.com/KratosMultiphysics/Kratos/issues/9779 Thank you @EduardGomezEscandell
Ping @KratosMultiphysics/implementation-committee
I'm also on board for const-correctness, but making that happen properly will be a pretty big undertaking and will definitely lead to extensive changes and API breaks. If we're doing it, we ought to do it right all at once to avoid breaking everyone's branches/forks all the time.
@KratosMultiphysics/technical-committee agrees about this change, however we think that @KratosMultiphysics/implementation-committee should take the lead on finishing it.
Feel free to start it from zero, this branch is probably a bit too old now.
If you chose to continue on this branch, or use it as inspiration, don't hesitate to ask about any of the changes I made (although I can't promise I'll remember the reasoning 😅).
Hey, we still want to merge this. reopening.
*Note: We want to keep the PR open so the branch does not get deleted by accident.
Oh I thought you were starting your implementation from scratch :sweat_smile:.
I was just trying to remove my old PRs.