astutils.cpp: optimized followAllReferences() a bit
#5442
Conversation
firewave
changed the title
followAlReferences() a bitfollowAllReferences() a bit
lib/astutils.cpp
Outdated
| if (argvar->isArgument() && (argvar->isReference() || argvar->isRValueReference())) { | ||
| const int n = getArgumentPos(argvar, f); | ||
| if (n < 0) { | ||
| break; |
There was a problem hiding this comment.
This changes the behavior. Instead of exiting function this will only exit the for loop.
There was a problem hiding this comment.
That is intentional. It exits the loop, result is empty and then it goes to the code at the end of the function...but that doesn't happen because I overlooked the outer loop over returns...well spotted.
|
Best use split view and "hide whitespaces" option to properly review this since the indentation changed. |
lib/astutils.cpp
Outdated
| er.emplace_back(tok->previous(), "Called function passing '" + argTok->expressionString() + "'."); | ||
| auto refs = | ||
| followAllReferences(argTok, temporary, inconclusive, std::move(er), depth - returns.size()); | ||
| if (!inconclusive && refs.size() > 1) { |
There was a problem hiding this comment.
This behavior for this and the following lines has also changed. Again I missed the outer loop so result might have been set in an earlier iteration.
But if we hit this block more than once we bailed out when the prerequisites were not met even though we might already have entries in result. If that should/can never happen or is unintentional we also need to exit after we added something to result.
There was a problem hiding this comment.
Or we should have continued instead of returned.
|
As this did not cause any tests to fail I wonder if we should have a test on a fixed source where we generate the ValueFlow debug output and compare it. Same for the AST/generated code. That would be quite massive but we would see the actual impact. |
|
I removed the last commit which had the problematic changes. The other changes already yield similar improvements and are safe. Will provide updated performance numbers soon. |
firewave
changed the title
followAllReferences() a bitfollowAllReferences() a bit
|
Using |
firewave
changed the title
followAllReferences() a bitfollowAllReferences() a bit
|
Ready for review - again. |
firewave
added
merge-after-next-release
4 participants
Scanning
common/file.cof thexrdpproject with--force --std=c11 --std=c++11 --inline-suppr --enable=warning:Clang 16
4,208,373,435->4,143,907,657Clang 16 (Boost)
3,837,285,621->3,609,164,192GCC 13
4,336,042,153->4,331,137,034GCC 13 (Boost)
3,896,319,383->3,795,013,995