Calculate token scopes in advance rather than as the tokenlist is iterated#2038
Conversation
Bringing fork up to date
|
The Travis CI build failed because CppCheck's output got too long, is that likely to be my change or an unrelated failure? |
We have seen such failures before. But hmm.. I do not think that this is an intermittent error. My guess is that it's somehow caused by your changes somehow. |
|
The log files are limited to 4 MB and the log output of the clang analysis with Cppcheck is very large (it is already nearly 4 MB in size). |
|
I would like to speedup Travis. So I have thought about moving the Clang analysis step. Maybe it could be done by a daca@home worker. What is the point really for the clang analysis on Travis? If it is to detect crashes, well I assume we get this info from daca@home nowadays? If we want to look at the warnings... then I definitely think that daca@home is better than Travis because we get a diff. |
|
I can only guess and think it is meant to detect problems like crashes. IMHO it is useful to have it in Travis because it could take some time to get this info from daca@home. In Travis it is not necessary to manually check a website and you see which commit exactly caused it. In daca@home some investigation could be necessary. But maybe it can be made faster by running the analysis only against a more limited number of source code files or so. To fix the too large log, Cppcheck could be run with the |
|
Sorry.. but I'll remove than clang analysis step. |
|
The thing I am worried about is that this somehow has unintended side effects. As far as I understand the changes are only meant to be a optimisation. Is changed behavior expected? |
|
There should be no change in the set of warnings found with this change. I'm running daca@home locally on this change now, I will go through the logs and look for any significant diffs. |
|
ok I merge this now again. |
|
I left daca@home running overnight, it found a small number of differences caused by this change. I'll analyse the results and make sure there isn't a problem. |
Do you have a script for running daca@home on branches/PRs or so which might be worth to commit (maybe in tools\test) or share otherwise? It could be improved or even integrated into the client later. |
|
I just hardcoded the donate-cpu-server script to use the commit hash of my fork before I made any changes, sorry. |
…rated (cppcheck-opensource#2038) * Removed redundant scope calculation * Add scope propagation code to insertToken * Add relevant scope code to Token class * Add code to calculate the scope of Tokens * Add calculateScopes method to class * Add missing include for shared_ptr
This is a duplicate of #1882, but with a bug fix that should fix the issues with missing warnings.
I have been running daca2 on this code locally, diffing it with 1.88, and found no significant missing issues. Can you remember which packages you saw problems from specifically? I will make sure I test on those.
The fix was to make sure that when adding a new scope name to an existing scope string, the new section is not empty, otherwise you end up with a scope name that ends in ' :: '.