removed CppCheck dependency from CppCheckExecutor::parseFromArgs()
#4967
Conversation
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
firewave
changed the title
CppCheck dependency from CppCheckExecutor::parseFromArgs()CppCheck dependencies in CppCheckExecutor
firewave
commented
Apr 14, 2023
cli/cppcheckexecutor.cpp
Outdated
| @@ -310,11 +303,15 @@ int CppCheckExecutor::check_internal(CppCheck& cppcheck) | |||
| AnalyzerInformation::writeFilesTxt(settings.buildDir, fileNames, settings.userDefines, settings.project.fileSettings); | |||
| } | |||
|
|
|||
| CppCheck cppcheck(*this, true, executeCommand); | |||
| cppcheck.settings() = settings; // this is a copy | |||
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.
firewave
changed the title
CppCheck dependencies in CppCheckExecutorCppCheck dependency from CppCheckExecutor::parseFromArgs()
|
I backed out the further refactoring as it doesn't work until the ownership has been changed. |
danmar
approved these changes
Apr 16, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.