◐ Shell
clean mode source ↗

clang-tidy.yml: updated to Clang 21 by firewave · Pull Request #421 · cppcheck-opensource/simplecpp

@firewave

/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
      |                 ^

@glankk this was introduced in 8f8bfe3 - please have a look

@glankk

@glankk this was introduced in 8f8bfe3 - please have a look

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.

@glankk

@firewave

@firewave

I worked around the bugprone-exception-escape warning for now and filed #537.

@firewave firewave marked this pull request as ready for review

September 9, 2025 10:29

firewave

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.

@chrchr-github

chrchr-github

}

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.

chrchr-github

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.

@chrchr-github

firewave


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.

firewave

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.

firewave

{
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.

@firewave

I validated the misc-const-correctness changes with #548 and the suppressions removed.

danmar

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.

@chrchr-github

Are there any plans for a new simplecpp version?

@firewave

Are there any plans for a new simplecpp version?

From my side after #475 was merged.