-
Notifications
You must be signed in to change notification settings - Fork 90
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
Return non-const should be preferred #2269
Conversation
Returning non-const objects has no disadvantage for const correctness and is easier to read.
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.
clang-tidy made some suggestions
@@ -288,14 +297,13 @@ private: | |||
* Shift a 2D field in Z. | |||
* Since 2D fields are constant in Z, this has no effect | |||
*/ | |||
const Field2D shiftZ(const Field2D& f, const Field2D& UNUSED(zangle), | |||
const std::string UNUSED(region) = "RGN_NOX") const { | |||
Field2D shiftZ(const Field2D& f, const Field2D& UNUSED(zangle), |
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.
warning: method shiftZ
can be made static [readability-convert-member-functions-to-static]
Field2D shiftZ(const Field2D& f, const Field2D& UNUSED(zangle),
^
static
const Field2D shiftZ(const Field2D& f, const Field2D& UNUSED(zangle), | ||
const std::string UNUSED(region) = "RGN_NOX") const { | ||
Field2D shiftZ(const Field2D& f, const Field2D& UNUSED(zangle), | ||
const std::string UNUSED(region) = "RGN_NOX") const { |
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.
warning: the const qualified parameter UNUSED_region
is copied for each invocation; consider making it a reference [performance-unnecessary-value-param]
const std::string UNUSED(region) = "RGN_NOX") const {
^
&
/github/workspace/include/unused.hxx:29:20: note: expanded from macro 'UNUSED'
# define UNUSED(x) UNUSED_ ## x __attribute__((unused))
^
note: expanded from here
const Field2D shiftZ(const Field2D& f, const Field2D& UNUSED(zangle), | ||
REGION UNUSED(region)) const { | ||
"const std::string& region = \"RGN_NOX\") instead")]] Field2D | ||
shiftZ(const Field2D& f, const Field2D& UNUSED(zangle), REGION UNUSED(region)) const { |
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.
warning: method shiftZ
can be made static [readability-convert-member-functions-to-static]
shiftZ(const Field2D& f, const Field2D& UNUSED(zangle), REGION UNUSED(region)) const {
^ ~~~~~~
const YDirectionType y_direction_out, | ||
const std::string& region = "RGN_NOX") const; | ||
Field3D shiftZ(const Field3D& f, const Tensor<dcomplex>& phs, | ||
const YDirectionType y_direction_out, |
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.
warning: parameter y_direction_out
is const-qualified in the function declaration; const-qualification of parameters only has an effect in function definitions [readability-avoid-const-params-in-decls]
const YDirectionType y_direction_out,
^~~~~~
const YDirectionType y_direction_out, | ||
const std::string& region = "RGN_NOX") const; | ||
FieldPerp shiftZ(const FieldPerp& f, const Tensor<dcomplex>& phs, | ||
const YDirectionType y_direction_out, |
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.
warning: parameter y_direction_out
is const-qualified in the function declaration; const-qualification of parameters only has an effect in function definitions [readability-avoid-const-params-in-decls]
const YDirectionType y_direction_out,
^~~~~~
Nice, thanks @dschwoerer ! |
Returning non-const objects has no disadvantage for const correctness
and is easier to read.
(as suggested by GHA bot)