Skip to content

Tokenizer: provide TokenList in constructor#7468

Merged
firewave merged 2 commits intocppcheck-opensource:mainfrom
firewave:toklist-123-x
May 1, 2025
Merged

Tokenizer: provide TokenList in constructor#7468
firewave merged 2 commits intocppcheck-opensource:mainfrom
firewave:toklist-123-x

Conversation

@firewave
Copy link
Copy Markdown
Collaborator

No description provided.

@firewave firewave marked this pull request as draft April 15, 2025 15:22
@firewave firewave marked this pull request as ready for review April 15, 2025 15:24
@firewave firewave marked this pull request as draft April 15, 2025 15:25
Comment thread lib/cppcheck.cpp
Comment on lines -1224 to +1233
ErrorMessage errmsg = ErrorMessage::fromInternalError(e, &tokenizer.list, file.spath());
ErrorMessage errmsg = ErrorMessage::fromInternalError(e, nullptr, file.spath());
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.

Since TokenList might throw InternalError it seem strange that we pass that object into the exception handling. Also it was no longer available.

Comment thread lib/cppcheck.cpp
Comment on lines +1200 to +1203
} catch (const InternalError &e) {
ErrorMessage errmsg = ErrorMessage::fromInternalError(e, &tokenizer.list, file.spath());
mErrorLogger.reportErr(errmsg);
}
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 multiple layers of exception handling seem excessive. I think it is possible to clean this up a bit but that requires test coverage first.

@firewave

This comment was marked as resolved.

@firewave firewave marked this pull request as ready for review April 23, 2025 18:49
@firewave firewave merged commit d8d4ea3 into cppcheck-opensource:main May 1, 2025
53 checks passed
@firewave firewave deleted the toklist-123-x branch May 1, 2025 12:18
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.

1 participant