Skip to content

Fix #13550 Remove boostForeachError/checkboost.cpp#7198

Merged
chrchr-github merged 13 commits intocppcheck-opensource:mainfrom
chrchr-github:chr_13472
Jan 13, 2025
Merged

Fix #13550 Remove boostForeachError/checkboost.cpp#7198
chrchr-github merged 13 commits intocppcheck-opensource:mainfrom
chrchr-github:chr_13472

Conversation

@chrchr-github
Copy link
Copy Markdown
Collaborator

@chrchr-github chrchr-github commented Jan 9, 2025

No description provided.

@chrchr-github chrchr-github changed the title Fix #13472, #13516 FP unreachableCode (GNU inline function, custom loop macro) Bail out for unknown loop macros and GNU inline functions Jan 9, 2025
@chrchr-github
Copy link
Copy Markdown
Collaborator Author

chrchr-github commented Jan 9, 2025 •
edited
Loading

We should probably just simplify BOOST_FOREACH to a range-based loop and do away with checkboost.cpp (already exists in boost.cfg).

@chrchr-github
Copy link
Copy Markdown
Collaborator Author

chrchr-github commented Jan 9, 2025

If we really want to support MSVC's for each (never heard of it), it should also be simplified better.
https://learn.microsoft.com/en-us/cpp/dotnet/for-each-in?view=msvc-170

@chrchr-github
Copy link
Copy Markdown
Collaborator Author

chrchr-github commented Jan 9, 2025

Invalid code: https://github.com/chrchr-github/cppcheck/blob/3d51420bad0d03cbb194ecc5227d95bba62e9cfd/test/testuninitvar.cpp#L2148

firewave
firewave reviewed Jan 9, 2025
View reviewed changes
Comment thread lib/tokenize.cpp
if (Token::simpleMatch(tok, "for each")) {
// 'for each ( )' -> 'asm ( )'
tok->str("asm");
// 'for each (x in y )' -> 'for (x : y)'
Copy link
Copy Markdown
Collaborator

@firewave firewave Jan 9, 2025

Choose a reason for hiding this comment

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

That's C++11 code. Does that work if we are using C++03 as standard?

Copy link
Copy Markdown
Collaborator Author

@chrchr-github chrchr-github Jan 9, 2025

Choose a reason for hiding this comment

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

I don't think we have any loop-handling code that is conditional on the standard. Also, the new simplification can only be an improvement on the asm hack.

Copy link
Copy Markdown
Collaborator

@firewave firewave Jan 9, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Definitely - especially since we are handling asm so well. It also now matches what the library configurations are doing.

@chrchr-github
Copy link
Copy Markdown
Collaborator Author

chrchr-github commented Jan 9, 2025

The test cases involving std::set are invalid, since it has stable insert/erase.

@chrchr-github chrchr-github changed the title Bail out for unknown loop macros and GNU inline functions Fix #13550 Remove boostForeachError/checkboost.cpp Jan 12, 2025
@chrchr-github chrchr-github marked this pull request as ready for review January 13, 2025 09:08
@chrchr-github
Copy link
Copy Markdown
Collaborator Author

chrchr-github commented Jan 13, 2025

There are no instances of boostForeachError in daca, so the impact should be minimal:
http://cppcheck1.osuosl.org:8000/head-boostForeachError

@chrchr-github chrchr-github merged commit 6ee9633 into cppcheck-opensource:main Jan 13, 2025
@chrchr-github chrchr-github deleted the chr_13472 branch January 13, 2025 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@firewave firewave firewave left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@chrchr-github @firewave