Uh oh!
There was an error while loading. Please reload this page.
removed CppCheck dependency from CppCheckExecutor::parseFromArgs() - #4967
Merged
Conversation
firewave
marked this pull request as draft
April 14, 2023 10:49
CppCheck dependency from CppCheckExecutor::parseFromArgs()CppCheck dependencies in CppCheckExecutorfirewave
marked this pull request as ready for review
April 14, 2023 10:59
firewave
commented
Apr 14, 2023
| } | ||
| CppCheck cppcheck(*this, true, executeCommand); | ||
| cppcheck.settings() = settings; // this is a copy |
CollaboratorAuthor
There was a problem hiding this comment.
This highlights the issue I want to resolve by moving the ownership out of CppCheck. We need to propagate back all the changes from the settings attached to CppCheck. In some cases we are not doing this correctly. This is why the settings should be immutable during the analysis so we don't have to do that.
Also we do that copy for each file with the multi-threaded implementation which is a hot spot if there are many libraries loaded.
CppCheck dependencies in CppCheckExecutorCppCheck dependency from CppCheckExecutor::parseFromArgs()firewave
commented
Apr 14, 2023
CollaboratorAuthor
I backed out the further refactoring as it doesn't work until the ownership has been changed. |
danmar
approved these changes
Apr 16, 2023
Uh oh!
There was an error while loading. Please reload this page.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
That function is basically just parsing the command-line options into the settings so no need for a
CppCheckobject yet. Also moved the object down to the actual usage.Preparation for moving the
Settingsownership out ofCppCheckin #4964.