Skip to content

Conversation

@firewave
Copy link
Collaborator

No description provided.

@firewave
Copy link
Collaborator Author

firewave commented Aug 5, 2025

/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
Copy link
Contributor

glankk commented Aug 14, 2025

@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
Copy link
Contributor

glankk commented Aug 14, 2025

Related: #493

@firewave
Copy link
Collaborator Author

firewave commented Sep 8, 2025

I filed https://trac.cppcheck.net/ticket/14119 about detecting one of the misc-const-correctness cases ourselves.

@firewave
Copy link
Collaborator Author

firewave commented Sep 9, 2025

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
Comment on lines +433 to 430
// NOLINTNEXTLINE(misc-const-correctness) - FP
FileData *const newdata = new FileData(std::move(data));
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I filed llvm/llvm-project#157730 about this.

@chrchr-github
Copy link
Contributor

Can this be merged? It is blocking danmar/cppcheck#7807

simplecpp.cpp Outdated
}

Token * const output_end_1 = output.back();
const Token * const output_end_1 = output.back();
Copy link
Contributor

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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Indeed.

Maybe I should apply danmar/cppcheck#4785 to simplecpp first.

Copy link
Collaborator Author

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;
Copy link
Contributor

@chrchr-github chrchr-github Sep 24, 2025

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.

Copy link
Collaborator Author

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.

Copy link
Collaborator Author

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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@chrchr-github
Copy link
Contributor

Ping @danmar


std::string type;
for (simplecpp::Token *typeToken = tok1; typeToken != tok2; typeToken = typeToken->next) {
for (const simplecpp::Token *typeToken = tok1; typeToken != tok2; typeToken = typeToken->next) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

False negative tracked as https://trac.cppcheck.net/ticket/14119.

if (tok->str() != "sizeof")
continue;
simplecpp::Token *tok1 = tok->next;
const simplecpp::Token *tok1 = tok->next;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

simplecpp.cpp Outdated
{
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) {
Copy link
Collaborator Author

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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

@firewave
Copy link
Collaborator Author

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

Copy link
Owner

@danmar danmar left a comment

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

simplecpp.cpp Outdated
void simplecpp::TokenList::constFoldQuestionOp(Token **tok1)
{
bool gotoTok1 = false;
// NOLINTNEXTLINE(misc-const-correctness) - technically correct but used to access non-cost data
Copy link
Owner

Choose a reason for hiding this comment

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

"non-cost" => "non-const"

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

simplecpp.cpp Outdated
}
}

// NOLINTNEXTLINE(misc-const-correctness) - technically correct but used to access non-cost data
Copy link
Owner

Choose a reason for hiding this comment

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

spelling: "non-cost"

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

@firewave firewave merged commit d86671f into danmar:master Sep 29, 2025
16 checks passed
@firewave firewave deleted the clang-21 branch September 29, 2025 00:35
@chrchr-github
Copy link
Contributor

Are there any plans for a new simplecpp version?

@firewave
Copy link
Collaborator Author

Are there any plans for a new simplecpp version?

From my side after #475 was merged.

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.

4 participants