cppcheck icon indicating copy to clipboard operation
cppcheck copied to clipboard

added (optional) lazy execution of ValueFlow

Open firewave opened this issue 3 years ago • 4 comments

firewave avatar Sep 29 '22 12:09 firewave

I think its a lot cleaner to use values() instead of mImpl->mValues. However, !mImp->mValues is not the equivalent of !values.empty(). If we use values() then the checks for null can be removed.

pfultz2 avatar Oct 04 '22 16:10 pfultz2

I think its a lot cleaner to use values() instead of mImpl->mValues.

That was my takeaway as well after I "cracked" this and saw only one method needs to access the raw pointers - which is quite nice. I will extract the encapsulation and cleanup parts of this into a separate PR but it still needs some work (see below).

However, !mImp->mValues is not the equivalent of !values.empty(). If we use values() then the checks for null can be removed.

I am aware of that. I already fixed up a few mistakes I made in earlier revisions.

But it is still very much WIP...

I still have to profile this with valueflow enabled and disabled.

Also the lazy execution doesn't save anything at all. Only when used in conjunction with #4362 it will actually do something since there's currently no way to control specific checks and even the special ones don't do that as evident by the above PR and several tickets I filed. This is a bigger thing and quite a can of worms but I have some ideas on how to incrementally address this to enhance the control and get rid of my hacks used for profiling and the CI. I was hoping that maybe some checks can be adjusted to "delay" usage of the vaueflow data so we could profit in more cases but without proper control about what check is run it won't do much in the end.

There's also some tests failing with the lazy execution. But I guess they just lack the lazy execution hook. Still it is interesting and maybe we should make valueflow more explicit in tests so we know what depends on it. It might also help with determining how to increase the test coverage. But I haven't looked into this at all.

firewave avatar Oct 04 '22 17:10 firewave

This should "help" (as in executing less code) with tests which use the Tokenizer but do not rely on the ValueFlow. That might also fix the issue I hit in #5299.

firewave avatar Aug 08 '23 22:08 firewave

I will try to pull out the wrapper function for values so this can finally progress.

firewave avatar Apr 20 '24 17:04 firewave