GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — clang-tidy.yml: updated to Clang 21 by firewave · Pull Request #421 · cppcheck-opensource/simplecpp

Conversation

@firewave

@firewave firewave commented Mar 17, 2025

Copy link
Copy Markdown
Collaborator

No description provided.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@firewave
firewave force-pushed the clang-21 branch from 8283561 to 9e8e7f2 Compare March 17, 2025 11:51
@firewave
firewave force-pushed the clang-21 branch 2 times, most recently from db52506 to 7babd3f Compare June 11, 2025 11:10
@firewave
firewave force-pushed the clang-21 branch from 7babd3f to ea3e62c Compare August 4, 2025 20:44
@firewave

firewave commented Aug 5, 2025

Copy link
Copy Markdown
Collaborator Author 
/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
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@glankk

glankk commented Aug 14, 2025

Copy link
Copy Markdown
Collaborator

@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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@glankk glankk mentioned this pull request Aug 14, 2025
Replace reference members with pointers #493
Open
@glankk

glankk commented Aug 14, 2025

Copy link
Copy Markdown
Collaborator

Related: #493
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@firewave

firewave commented Sep 8, 2025

Copy link
Copy Markdown
Collaborator Author

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@firewave firewave mentioned this pull request Sep 8, 2025
Fix #14119 FN constVariablePointer for variable declared in for loop / partial fix for #14148 cppcheck-opensource/cppcheck#7807
Merged
@firewave
firewave force-pushed the clang-21 branch from 9362069 to 092f92f Compare September 9, 2025 10:18
@firewave firewave mentioned this pull request Sep 9, 2025
address bugprone-exception-escape clang-tidy warning in Macro copy constructor #537
Open
@firewave

firewave commented Sep 9, 2025 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Collaborator Author

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@firewave
firewave force-pushed the clang-21 branch from 495a113 to 63cdcfb Compare September 9, 2025 10:28
@firewave
firewave marked this pull request as ready for review September 9, 2025 10:29
firewave
firewave commented Sep 9, 2025
View reviewed changes
Comment thread simplecpp.h
Comment on lines +433 to 434
// NOLINTNEXTLINE(misc-const-correctness) - FP
FileData *const newdata = new FileData(std::move(data));

@firewave firewave Sep 9, 2025

Copy link
Copy Markdown
Collaborator Author

There was a problem hiding this comment.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@firewave
firewave force-pushed the clang-21 branch from 63cdcfb to 95088dc Compare September 10, 2025 12:46
@chrchr-github

chrchr-github commented Sep 23, 2025

Copy link
Copy Markdown
Contributor

Can this be merged? It is blocking cppcheck-opensource/cppcheck#7807
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
chrchr-github
chrchr-github reviewed Sep 24, 2025
View reviewed changes
Comment thread simplecpp.cpp Outdated
}

Token * const output_end_1 = output.back();
const Token * const output_end_1 = output.back();

@chrchr-github chrchr-github Sep 24, 2025

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@firewave firewave Sep 24, 2025

Copy link
Copy Markdown
Collaborator Author

There was a problem hiding this comment.

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 cppcheck-opensource/cppcheck#4785 to simplecpp first.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@firewave firewave Sep 24, 2025

Copy link
Copy Markdown
Collaborator Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Done.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
chrchr-github
chrchr-github reviewed Sep 24, 2025
View reviewed changes
Comment thread simplecpp.cpp
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;

@chrchr-github chrchr-github Sep 24, 2025 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@firewave firewave Sep 24, 2025

Copy link
Copy Markdown
Collaborator Author

There was a problem hiding this comment.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@firewave firewave Sep 27, 2025

Copy link
Copy Markdown
Collaborator Author

There was a problem hiding this comment.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@firewave firewave Sep 27, 2025

Copy link
Copy Markdown
Collaborator Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

This is the same as https://trac.cppcheck.net/ticket/14161.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@firewave
firewave force-pushed the clang-21 branch from 95088dc to f595d1e Compare September 24, 2025 17:35
@chrchr-github

chrchr-github commented Sep 26, 2025

Copy link
Copy Markdown
Contributor

Ping @danmar
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
firewave
firewave commented Sep 27, 2025
View reviewed changes
Comment thread simplecpp.cpp

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

@firewave firewave Sep 27, 2025

Copy link
Copy Markdown
Collaborator Author

There was a problem hiding this comment.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
firewave
firewave commented Sep 27, 2025
View reviewed changes
Comment thread simplecpp.cpp
if (tok->str() != "sizeof")
continue;
simplecpp::Token *tok1 = tok->next;
const simplecpp::Token *tok1 = tok->next;

@firewave firewave Sep 27, 2025

Copy link
Copy Markdown
Collaborator Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I filed https://trac.cppcheck.net/ticket/14161 about this.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
firewave
firewave commented Sep 27, 2025
View reviewed changes
Comment thread 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) {

@firewave firewave Sep 27, 2025

Copy link
Copy Markdown
Collaborator Author

There was a problem hiding this comment.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@firewave firewave Sep 27, 2025

Copy link
Copy Markdown
Collaborator Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Done.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@firewave
firewave force-pushed the clang-21 branch from f595d1e to 28b4009 Compare September 27, 2025 08:46
@firewave

firewave commented Sep 27, 2025

Copy link
Copy Markdown
Collaborator Author

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
danmar
danmar approved these changes Sep 27, 2025
View reviewed changes

@danmar danmar left a comment

Copy link
Copy Markdown
Collaborator

There was a problem hiding this 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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread 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

@danmar danmar Sep 27, 2025

Copy link
Copy Markdown
Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@firewave firewave Sep 29, 2025

Copy link
Copy Markdown
Collaborator Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Done.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread simplecpp.cpp Outdated
}
}

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

@danmar danmar Sep 27, 2025

Copy link
Copy Markdown
Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

spelling: "non-cost"

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@firewave firewave Sep 29, 2025

Copy link
Copy Markdown
Collaborator Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Done.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
firewave added 2 commits September 29, 2025 02:25
@firewave
clang-tidy.yml: updated to Clang 21
64df78a
@firewave
mitigated misc-const-correctness clang-tidy warnings
e982bd8
firewave added 6 commits September 29, 2025 02:26
@firewave
disabled -Wnrvo Clang warning
36b4c0c
@firewave
split up Clang warnings in CMake
446663d
@firewave
.clang-tidy: disabled readability-use-concise-preprocessor-directives
197fb48
@firewave
fixed readability-math-missing-parentheses clang-tidy warnings
a43105f
@firewave
mitigated misc-const-correctness clang-tidy false positive
0e31c6a
@firewave
avoid bugprone-exception-escape in Macro copy constructor
9af1b83
@firewave
firewave force-pushed the clang-21 branch from 28b4009 to 9af1b83 Compare September 29, 2025 00:26
@firewave
firewave merged commit d86671f into cppcheck-opensource:master Sep 29, 2025
1 check passed

Uh oh!

There was an error while loading. Please reload this page.
@firewave
firewave deleted the clang-21 branch September 29, 2025 00:35
@chrchr-github

chrchr-github commented Sep 29, 2025

Copy link
Copy Markdown
Contributor

Are there any plans for a new simplecpp version?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@firewave

firewave commented Sep 29, 2025

Copy link
Copy Markdown
Collaborator Author

Are there any plans for a new simplecpp version?

From my side after #475 was merged.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@danmar danmar danmar approved these changes
+1 more reviewer
@chrchr-github chrchr-github chrchr-github left review comments
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@firewave @glankk @chrchr-github @danmar
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.