clang-tidy.yml: updated to Clang 21 by firewave · Pull Request #421 · cppcheck-opensource/simplecpp
/usr/bin/../lib/gcc/x86_64-linux-gnu/14/../../../../include/c++/14/bits/stl_pair.h:295:17: error: an exception may be thrown in function 'pair' which should not throw exceptions [bugprone-exception-escape,-warnings-as-errors]
295 | constexpr pair(pair&&) = default; ///< Move constructor
| ^
/home/runner/work/simplecpp/simplecpp/simplecpp.cpp:1754:29: note: frame #0: unhandled exception of type 'simplecpp::Macro::Error' may be thrown in function 'parseDefine' here
1754 | throw Error(tok->location, "In definition of '" + nameTokDef->str() + "': Missing opening parenthesis for __VA_OPT__");
| ^
/home/runner/work/simplecpp/simplecpp/simplecpp.cpp:1515:21: note: frame #1: function 'operator=' calls function 'parseDefine' here
1515 | parseDefine(other.nameTokDef);
| ^
/home/runner/work/simplecpp/simplecpp/simplecpp.cpp:1502:19: note: frame #2: function 'Macro' calls function 'operator=' here
1502 | *this = other;
| ^
/usr/bin/../lib/gcc/x86_64-linux-gnu/14/../../../../include/c++/14/bits/stl_pair.h:295:17: note: frame #3: function 'pair' calls function 'Macro' here
295 | constexpr pair(pair&&) = default; ///< Move constructor
| ^
The root cause of this issue seems to be that Macro doesn't have a move constructor, so its copy constructor is used instead in the std::pair move constructor. The default move constructor in Macro is deleted because it has a reference as a member (which I think is generally a bad idea).
I think the cleanest solution to this is to replace the references with pointers and implement sane move operators. I'll work out a draft. @danmar please chime in if you have an opinion on this.
I worked around the bugprone-exception-escape warning for now and filed #537.
firewave
marked this pull request as ready for review
Comment on lines +433 to 434
| // NOLINTNEXTLINE(misc-const-correctness) - FP | ||
| FileData *const newdata = new FileData(std::move(data)); |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| } | ||
|
|
||
| Token * const output_end_1 = output.back(); | ||
| const Token * const output_end_1 = output.back(); |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not semantically const. Maybe a NOLINT would be more appropriate.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
| const bool unexpectedA = (!A->name && !A->number && !A->str().empty() && !canBeConcatenatedWithEqual && !canBeConcatenatedStringOrChar); | ||
|
|
||
| Token * const B = tok->next->next; | ||
| const Token * const B = tok->next->next; |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same, B's members are modified below.
Looks like there still is a cppcheck FN here.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's possible. I did not check all remaining cases yet.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's several false negatives.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
|
||
| std::string type; | ||
| for (simplecpp::Token *typeToken = tok1; typeToken != tok2; typeToken = typeToken->next) { | ||
| for (const simplecpp::Token *typeToken = tok1; typeToken != tok2; typeToken = typeToken->next) { |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| if (tok->str() != "sizeof") | ||
| continue; | ||
| simplecpp::Token *tok1 = tok->next; | ||
| const simplecpp::Token *tok1 = tok->next; |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| { | ||
| bool gotoTok1 = false; | ||
| for (Token *tok = *tok1; tok && tok->op != ')'; tok = gotoTok1 ? *tok1 : tok->next) { | ||
| for (const Token *tok = *tok1; tok && tok->op != ')'; tok = gotoTok1 ? *tok1 : tok->next) { |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is also only technically correct.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
I validated the misc-const-correctness changes with #548 and the suppressions removed.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just have spelling to complain about
| void simplecpp::TokenList::constFoldQuestionOp(Token **tok1) | ||
| { | ||
| bool gotoTok1 = false; | ||
| // NOLINTNEXTLINE(misc-const-correctness) - technically correct but used to access non-cost data |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"non-cost" => "non-const"
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
| } | ||
| } | ||
|
|
||
| // NOLINTNEXTLINE(misc-const-correctness) - technically correct but used to access non-cost data |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
spelling: "non-cost"
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
Are there any plans for a new simplecpp version?
From my side after #475 was merged.