Template argument deduction using SymbolDatabase#8720
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| case ValueType::UNKNOWN_INT: | ||
| // not deducible | ||
| return DeducedType(); | ||
| } |
|
So I ran some numbers comparing the time with valueflow disabled:
It seems to be about ~20-30% slower, which is much better than what it was before at ~80% slower doing full rebuilds. Partial rebuilds seem to really help a lot with this. Obviously since we are doing more instantiations than before it is going to be slower even as there may be room to improve this further. I think this is an acceptable slowdown and in the end I think this will still be faster than the approach in #8688 as we will have to always compute the valuetypes twice since they are seperate components. So I think this approach is ultimately better. @danmar @chrchr-github What are you thoughts on this approach? I can work on cleaning up this PR and fixing the CI failures if you think this is the better way to go. |
| // the class itself, then its transitive base classes | ||
| void addClassLevels(const Scope* classScope) | ||
| { | ||
| for (const Scope* scope : classScope->findAssociatedScopes()) |
| const auto it = parsed.find(&candidate); | ||
| if (it != parsed.cend()) | ||
| return it->second; | ||
| return parsed.emplace(&candidate, parseDeductionCandidate(candidate)).first->second; |
| const auto it = parsed.find(&candidate); | ||
| if (it != parsed.cend()) | ||
| return it->second; | ||
| return parsed.emplace(&candidate, parseDeductionCandidate(candidate)).first->second; |
|
@danmar @chrchr-github I think this is ready, if you want to review this. The one CI failure is network issue unrelated and the cppcheck premuim error is a false positive. |
|
It feels like ~30% slower analysis is quite a lot! :-( Do you think it will cause similar slowdown for C code or C++ code without templates? Technically I think this sounds promising. And more or less how I envision it should work (I don't have a clear idea, but I understand that we will need to perform ast/symboldatabase/valuetype/template instantiations together to get proper output). And doing it incrementally somehow sounds better... |
For Cppcheck source code does this make a difference in the parsing output? We don't use that much templates.. Could it be possible to run simpler and faster parsing if there are no templates (i.e. C code) and avoid the speed penalty? |
3 participants
This creates a loop where we run the TemplateSimplifier -> varids/links/AST/SymbolDatabase/ValueTypes, so on the second round of
TemplateSimplifierwe can use type information from theSymbolDatabase. Not all the passes need to run withTemplateSimpliferso there is aSymbolDatabase::finalize().On a naive run, it would delete all the AST/SymbolDatabase information, but this can be kind of slow. So I added some incremental updates by tracking where new tokens are added(plus the function bodies around changed call sites), and then refactoring functions to work on token ranges, so there is now
TokenList::createAst(start, end)and in SymbolDatabase:addSymbolsForNewTokenRanges(),updateFunctionAndVariablePointers(), a range-limitedsetValueTypeInTokenList()andfindAllScopes(const Token* startToken, const Token* endToken, Scope* startScope). Also thecreateSymbolDatabaseFindAllScopesinternals were split into reusable per-scope/per-function helpers to support this.Now this only does incremental updates on function instantiation because its much simpler to do as it only adds a
Function, its scope and locals(and the only stale references in old code are the renamed call sites, whichupdateFunctionAndVariablePointers()re-resolves by name). Instantiating a class introduces new functions and types that could be resolved in other parts of the code. So for this case it does a full rebuild.Also the template alias simplifications requires a full build as well as it restructures existing tokens throughout the list, not just in new ranges. So the "unchanged tokens keep valid info" premise of the incremental update no longer holds.
We can probably address these in the future but will require a larger refactor. I also added a
--template-full-rebuildso we can debug any issue with the incremental updates.