Skip to content

Conversation

@firewave
Copy link
Collaborator

No description provided.

@firewave
Copy link
Collaborator Author

CC @glankk

@firewave firewave marked this pull request as draft October 20, 2025 15:34
@firewave
Copy link
Collaborator Author

This affects the implementation in Cppcheck. Still working on integrating it.

Comment on lines -82 to +86

Location &operator=(const Location &other) {
if (this != &other) {
fileIndex = other.fileIndex;
line = other.line;
col = other.col;
}
return *this;
}
Location &operator=(const Location &other) = default;
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 was lacking the assignment of files. I have no idea how that could have ever been a valid object - beside the fact that containing a reference is even problematic to begin with.

@firewave
Copy link
Collaborator Author

Should not be merged before we post a release with the MinGW fixes.

I also want to get in some preprocessor cleanups on the Cppcheck side first.

@firewave firewave changed the title refs #424 - do not store reference to filelist in simplecpp::Location / added simplecpp::TokenList::file() / cleanups refs #424 / fixes #589 - do not store reference to filelist in simplecpp::Location / added simplecpp::TokenList::file() / cleanups Nov 14, 2025
@danmar
Copy link
Owner

danmar commented Nov 17, 2025

if this fixes a crash in cppcheck I am pretty interested to get this in. what do you think must happen before we can merge this PR ? It is still in a draft state.

@firewave
Copy link
Collaborator Author

To land this downstream it requires changes to the Cppcheck code which I still haven't finished locally as I keep running into issues, low quality code and missing test coverage. That is what all the preprocessor fixing and refactoring is all about. I am not sure how long it will take since I am not sure on how to fix everything along the way yet.

But it might be that this crash will become latent by one of the downstream fixes. I have not verified that yet.

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.

2 participants