Calculate token scopes in advance rather than as the tokenlist is iterated#1882
Conversation
Moved ScopeInfo2 declaration here as well because that's where it needs to be now.
This replaces the methods in the TemplateSimplifier which calculate the current scope as the token list is iterated. The old method required checking if the scope had changed for every token multiple times (for multiple iterations), which was surprisingly costly. Calculating scopes in advance like this decreases runtime on a worst-case file by around thirty percent. ScopeInfo objects are disposed of when the TemplateSimplification is done as they are not used later.
This has been replaced by code that calculates the scopes up front and stores them with each token, which is much faster.
|
The AppVeyor build timed out rather than failing with a specific error. Can it be retried? |
Sorry for late reply... I rebuild it now.. I wish I could give all contributors the right to rebuild appveyor. |
|
The template simplifier appends new tokens and copies tokens. Do these new tokens get a correct scope info? The new tokens could introduce new scopes so just inheriting the scope info of the previous token before the insertion may not be correct. I think you will need to recalculate the scope info for inserted tokens starting from the previous token. |
|
I thought that might be the case, but I wasn't sure. Just copying the cope of the previous token passed all of the tests, which is why I left it like that, but I'll have a look at fixing it. |
As the templatesimplificator expands templates and does multiple passes it needs to make sure all scopes are calculated.
This is now done properly when scopes are calculated.
This is now done by the TemplateSimplifier.
|
It turns out that the latest change in this pull request wipes out the performance gains, I'll fix it :) |
I thought that this might be useful for calculating scopes when Tokens are created, but as there are several ways of creating Tokens that don't guarantee that they are placed in a list it is easier to just calculate scopes when you know you have a list and when you know you're adding to a list.
Remove the global scopes collection, and clean them up instead by iterating through the tokenlist to find them. This means scopes can be calculated by functions in the Token class as well as in the Tokenizer class without leaking the scope object. Fix a couple of bugs in the calculateScopes method and make it more efficient.
Calculating scopes during insertToken only needs to happen if we created a new Token.
|
I'm going to need to spend some time working out why these tests are failing. The failure is a test I haven't merged into my copy yet, and there are a few changes required for it to work. What do you think of the code I've added, assuming I make it work with the merged changed? |
The code is valid code that compiles with clang, and was reduced from the Boost.MP11 library. It looks from the output that it is no longer expanding |
This fixes a bug caused by a statement matching 'struct B < 0 > ;'
This means ScopeInfo2 objects get properly cleaned up when they are no longer needed.
This has been replaced by shared_ptr for ScopeInfo2 objects
|
@danmar I'm happy with this change now. What do you think of it? |
|
Also how likely is it that this change will get into 1.88? |
sorry but this feels risky to me. So I'll merge after the release. |
…rated (cppcheck-opensource#1882) * Added scopeinfo member to token class Moved ScopeInfo2 declaration here as well because that's where it needs to be now. * Added scopeinfo accessors and declaration to class * Add new method for calculating scopes This replaces the methods in the TemplateSimplifier which calculate the current scope as the token list is iterated. The old method required checking if the scope had changed for every token multiple times (for multiple iterations), which was surprisingly costly. Calculating scopes in advance like this decreases runtime on a worst-case file by around thirty percent. ScopeInfo objects are disposed of when the TemplateSimplification is done as they are not used later. * Add calculateScopes method to header * Removed code that calculated current scope This has been replaced by code that calculates the scopes up front and stores them with each token, which is much faster. * Fixed compile errors from extra parentheses * Added missing code to fix memory leak * Added code to actually clean up ScopeInfo structs * Tidy up a dodgy for loop * Convert argument to const ref * Calculate missing scopes As the templatesimplificator expands templates and does multiple passes it needs to make sure all scopes are calculated. * Remove copying the scope to the next token This is now done properly when scopes are calculated. * Remove call to calculateScopes This is now done by the TemplateSimplifier. * Recalculate scopes for every pass of simplifyTemplates * Add code to calculate extra scopes as they are added I thought that this might be useful for calculating scopes when Tokens are created, but as there are several ways of creating Tokens that don't guarantee that they are placed in a list it is easier to just calculate scopes when you know you have a list and when you know you're adding to a list. * Fix several bugs and poorly designed code Remove the global scopes collection, and clean them up instead by iterating through the tokenlist to find them. This means scopes can be calculated by functions in the Token class as well as in the Tokenizer class without leaking the scope object. Fix a couple of bugs in the calculateScopes method and make it more efficient. * Remove unnecessary calls to calculateScopes * Move brace to correct position Calculating scopes during insertToken only needs to happen if we created a new Token. * Handle 'using namespace' declarations separately This fixes a bug caused by a statement matching 'struct B < 0 > ;' * Fix argument name mismatch * Actually use newScopeInfo when inserting Token * Switch to using shared_ptr to hold scopeInfos This means ScopeInfo2 objects get properly cleaned up when they are no longer needed. * Change ScopeInfo member to be a shared_ptr * Update code to use shared_ptr * Add missing include for shared_ptr * Remove unnecessary cleanup code This has been replaced by shared_ptr for ScopeInfo2 objects
…t is iterated (cppcheck-opensource#1882)" This reverts commit 0d7836f.
This is much more efficient, saving around a minute on a three minute run.
I think this code could be used elsewhere given the existence of ScopeInfo2 and ScopeInfo3, but I don't know the codebase well enough to say for sure.