Skip to content

Revert recent changes in checkConstVariable(), add tests (refs #12203)#5696

Merged
chrchr-github merged 6 commits intocppcheck-opensource:mainfrom
chrchr-github:chr_Fix12203
Nov 22, 2023
Merged

Revert recent changes in checkConstVariable(), add tests (refs #12203)#5696
chrchr-github merged 6 commits intocppcheck-opensource:mainfrom
chrchr-github:chr_Fix12203

Conversation

@chrchr-github
Copy link
Copy Markdown
Collaborator

No description provided.

Comment thread test/testother.cpp
errout.str());

check("struct S {\n"
" void f(int& r) { p = &r; }\n"
Copy link
Copy Markdown
Contributor

@pfultz2 pfultz2 Nov 22, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think isVariableChanged can be updated to handle this case. We are already doing something similiar for references.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would not even expect isVariableChanged() to return true here.

Comment thread test/testother.cpp
" int* p;\n"
"};\n"
"void g(std::vector<int>& v1, std::vector<int*>& v2) {\n"
" std::transform(v1.begin(), v1.end(), v2.begin(), [](auto& x) { return &x; });\n"
Copy link
Copy Markdown
Contributor

@pfultz2 pfultz2 Nov 22, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can't be handled by isVariableChanged. We need to check if it escapes. Ideally it would be nice to write a generic function(maybe escapes or referenceEscapes?) to check for this(with an indirect parameter for pointers and references), this should help to simplify the logic for this checker(and maybe others).

@chrchr-github chrchr-github merged commit 331db40 into cppcheck-opensource:main Nov 22, 2023
@chrchr-github chrchr-github deleted the chr_Fix12203 branch November 22, 2023 18:21
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