Skip to content

do not pass POD types by reference (based on clazy function-args-by-value check)#5388

Merged
danmar merged 2 commits intocppcheck-opensource:mainfrom
firewave:clazy-2-x
Sep 11, 2023
Merged

do not pass POD types by reference (based on clazy function-args-by-value check)#5388
danmar merged 2 commits intocppcheck-opensource:mainfrom
firewave:clazy-2-x

Conversation

@firewave
Copy link
Copy Markdown
Collaborator

@firewave firewave commented Sep 1, 2023

No description provided.

@firewave
Copy link
Copy Markdown
Collaborator Author

firewave commented Sep 1, 2023

I already filed https://trac.cppcheck.net/ticket/11880 about detecting this ourselves.

Comment thread lib/valueflow.cpp Outdated
Comment thread lib/color.h
@firewave firewave force-pushed the clazy-2-x branch 2 times, most recently from 7c4941e to c4b9b7c Compare September 1, 2023 15:37
@firewave
Copy link
Copy Markdown
Collaborator Author

firewave commented Sep 1, 2023

The naming-var inconsistency is already tracked in https://trac.cppcheck.net/ticket/11210.

@danmar
Copy link
Copy Markdown
Collaborator

danmar commented Sep 11, 2023

how do we prevent that clazy is executed something like this perhaps?

#ifdef __clazy__
#error Do not run clazy
#endif

?

isn't it better that arguments are const, how does that hurt?

@firewave
Copy link
Copy Markdown
Collaborator Author

how do we prevent that clazy is executed something like this perhaps?

#ifdef __clazy__
#error Do not run clazy
#endif

No idea.

isn't it better that arguments are const, how does that hurt?

See #5388 (comment).

@danmar danmar merged commit 02b836b into cppcheck-opensource:main Sep 11, 2023
@danmar
Copy link
Copy Markdown
Collaborator

danmar commented Sep 11, 2023

I do not feel sure about removing constness from arguments. But at the same time I'm not sure if I would want to enforce constness everywhere neither.

@firewave
Copy link
Copy Markdown
Collaborator Author

I do not feel sure about removing constness from arguments.

I would have kept it but then we should have cleaned up the headers to avoid potential confusion. But as these are POD types it should be clear but only if you have an IDE with hints (which is not a given). But as you see, we were not treating them as such.

But at the same time I'm not sure if I would want to enforce constness everywhere neither.

Since it is just POD types there's no strings attached. Everything else is a whole can of worms which need proper tooling support which approaches it from multiple angles at once or you will end up with potentially suboptimal code you will never detect.

@firewave firewave deleted the clazy-2-x branch September 11, 2023 18:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants