Skip to content

testrunner: more Tokenizer related refactoring#7440

Merged
firewave merged 4 commits intocppcheck-opensource:mainfrom
firewave:toklist-zzz
Apr 13, 2025
Merged

testrunner: more Tokenizer related refactoring#7440
firewave merged 4 commits intocppcheck-opensource:mainfrom
firewave:toklist-zzz

Conversation

@firewave
Copy link
Copy Markdown
Collaborator

@firewave firewave commented Apr 8, 2025

No description provided.

@firewave
Copy link
Copy Markdown
Collaborator Author

firewave commented Apr 8, 2025

This does not seem like much of a cleanups (especially because of the added noise in the Makefile) but this is about about getting rid of redundant stuff which complicates things as I try to untangle this language mess. As mentioned before the application code is fine but all these custom approaches in the tests complicate things.

This stuff really makes my head spin and every time I think I finally cracked this I reach another point where it seems like I have to rip it out all at once - which I want to avoid.

Comment on lines -1591 to +1586
ASSERT(startsWith(errout_str(), "[test.cpp:6]: (debug) Failed to parse 'using C = S < S < S < int"));
TODO_ASSERT(startsWith(errout_str(), "[test.cpp:6]: (debug) Failed to parse 'using C = S < S < S < int"));
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@chrchr-github This was producing an error because the code contained the preprocessed and non-preprocessed source because it was tokenized twice (I tracked it back to the introduction and it has always been that way). I think this is currently fine because it was about a hang/crash but if we expect to test for the error we might need to add another test.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

The original code was added in #5018.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

What is errout_str() after the change? I agree the output was not important, since the issue was a hang.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

It is empty.

Copy link
Copy Markdown
Collaborator

@chrchr-github chrchr-github Apr 11, 2025

Choose a reason for hiding this comment

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

That even seems like progress. But then I don't understand why the refactoring made the debug message go away.
If it's related to what you wrote above, just assert on the empty errout_str.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

But then I don't understand why the refactoring made the debug message go away.

See previous comment.

This was producing an error because the code contained the preprocessed and non-preprocessed source because it was tokenized twice

I was asking because I think we should have a test which actually detected the debug message.

@firewave firewave force-pushed the toklist-zzz branch 2 times, most recently from 06a5d32 to 55dc79f Compare April 8, 2025 10:45
@firewave firewave changed the title testrunner: more Tokenizer related cleanups testrunner: more Tokenizer related refactoring Apr 8, 2025
@firewave firewave marked this pull request as draft April 11, 2025 06:49
@firewave

This comment was marked as resolved.

@firewave firewave marked this pull request as ready for review April 11, 2025 07:04
@firewave firewave merged commit 5ba1c9d into cppcheck-opensource:main Apr 13, 2025
53 checks passed
@firewave firewave deleted the toklist-zzz branch April 13, 2025 12:47
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.

3 participants