Uh oh!
There was an error while loading. Please reload this page.
astutils.cpp: optimized followAllReferences() a bit - #5442
Conversation
followAlReferences() a bitfollowAllReferences() a bit| 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.
firewave
commented
Sep 12, 2023
Best use split view and "hide whitespaces" option to properly review this since the indentation changed. |
| 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.
firewave
commented
Sep 12, 2023
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. |
firewave
commented
Nov 8, 2023
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. |
followAllReferences() a bitfollowAllReferences() a bitUh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
firewave
commented
Nov 16, 2023
Using |
followAllReferences() a bitfollowAllReferences() a bitfirewave
commented
Dec 2, 2023
Ready for review - again. |
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