C++: Use std::array as-is wherever possible. Improve parameter passing on ctor/assignment#3936
Merged
C++: Use std::array as-is wherever possible. Improve parameter passing on ctor/assignment#3936
Conversation
emilk
reviewed
Oct 20, 2023
Member
Author
|
@emilk your comment encouraged me to be more thorough about this and "simply" implement the ISO C++ core guidelines (even though I think they're a lil bit silly) https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rf-in bright side is that the whole area is way more systematic now |
Base automatically changed from
andreas/cpp/fix-rotation-serialization
to
main
October 20, 2023 09:01
46d333d to
58653fe
Compare
Member
Author
|
discussed on slack and landed on this simpler approach which is very close to what we had to begin with. But cleaned up code, closed gaps and expanded documentation on the rationale for this |
emilk
reviewed
Oct 20, 2023
emilk
approved these changes
Oct 20, 2023
Member
|
I'll fix my comments |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Clarification: This PR is mostly scratching an itch of mine & addresses (in my experience) mostly aesthetic problems
Using std::array makes a bunch of codegen things easier. This in turn made it easy to revisit how parameters are passed on constructors.
Previous rationale on now discarded "more like cpp guidelines" approach:
This follows now the usual "common sense" logic that I believe people expect from C++ constructors:
The latter one is what we had exclusively before. It's less common in the wild which is why codegen has a bit of rationale in the comments about it.
You might say: "But @Wumpf! Passing large fixed size arrays or structs not by value is important as it avoids a copy, this isn't just aesthetic!". My reply being that from a language lawyer perspective this might be true, but practically there is no such thing as passing structs by value as calling conventions will almost always force (sufficiently large) structs to be passed by reference (with some exceptions to the rule to be fair). Meaning the compiler has a very easy time to optimize both caller and callee even without any inlining what-so-ever ... but I digress... ;)
Checklist