added (optional) lazy execution of ValueFlow by firewave · Pull Request #4521 · cppcheck-opensource/cppcheck · GitHub

No description provided.

These two variables can be moved into the mValuesFunc.

We could do something similar to what we do for memoize:

template<class F>
static std::function<void()> once(F f)
{
    bool ran = false;
    return [=]() mutable {
        if (ran)
            return;
        ran = true;
        f();
    };
}

This was very hacky. I just tried to get rid what I wanted it to do.

These two variables can be moved into the mValuesFunc.

Good point.

We could do something similar to what we do for [memoize]

Will take a look. I know there are patterns for this but I rarely use them so I keep forgetting them.

values().empty() is redundant here.

I kept it to avoid the function calls (which obviously won't do much) since some of these might be called loooooots of times. I haven't profiled any of this yet.

I would not call it "redundant" but "unnecessary". Still needs to be profiled.

values().empty() is redundant here.

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.

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.

…nt variable

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.

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

I filed https://trac.cppcheck.net/ticket/14243 about this.

firewave force-pushed the vf branch 3 times, most recently from 2b93752 to 07ba92f Compare September 29, 2022 13:26

pfultz2 reviewed Oct 4, 2022

View reviewed changes

firewave added 4 commits October 5, 2022 11:11

Token: avoid unnecessary direct access of TokenImpl::mValues

dfa2fb3

moved TokensFrontBack from token.h to tokenlist.h

c4f1685

tokenize.cpp: small refactoring

63e12eb

added lazy execution of ValueFlow via DISABLE_VALUEFLOW=2 environme…

58dcddb

…nt variable

firewave force-pushed the vf branch from 07ba92f to 58dcddb Compare October 5, 2022 09:19

firewave mentioned this pull request Apr 17, 2023

Add a --check-level option #4942

Merged

firewave mentioned this pull request Sep 1, 2023

avoid duplicated workflow invocations for pushes in PRs made from root repository branches #5377

Merged

firewave mentioned this pull request Oct 13, 2023

moved TokensFrontBack from token.h to tokenlist.h #5547

Merged

firewave mentioned this pull request Jan 5, 2024

enabled and mitigated debug warnings in cfg tests #5840

Merged

firewave mentioned this pull request Apr 19, 2024

Add --check-level=fast option that reuses much faster valueflow analysis from 1.90 #6097

Closed

                       return false;
-                  return mImpl->mValues &&
-                         std::any_of(mImpl->mValues->begin(), mImpl->mValues->end(), [&](const ValueFlow::Value& value) {
+                  return !values().empty() &&

+                  std::function<void()> mValuesFunc;
+                  mutable bool mValuesSet;
+                  mutable bool mInSetValues;

               const ValueFlow::Value* Token::getContainerSizeValue(const MathLib::bigint val) const
               {
-                  if (!mImpl->mValues)
+                  if (values().empty())

               bool Token::hasKnownValue() const
               {
-                  return mImpl->mValues && std::any_of(mImpl->mValues->begin(), mImpl->mValues->end(), std::mem_fn(&ValueFlow::Value::isKnown));
+                  return !values().empty() && std::any_of(values().begin(), values().end(), std::mem_fn(&ValueFlow::Value::isKnown));

               const ValueFlow::Value * Token::getInvalidValue(const Token *ftok, nonneg int argnr, const Settings *settings) const
               {
-                  if (!mImpl->mValues || !settings)
+                  if (values().empty() || !settings)

added (optional) lazy execution of ValueFlow by firewave · Pull Request #4521 · cppcheck-opensource/cppcheck · GitHub

Uh oh!

added (optional) lazy execution of ValueFlow - #4521

added (optional) lazy execution of ValueFlow#4521firewave wants to merge 4 commits intocppcheck-opensource:maincppcheck-opensource/cppcheck:mainfrom firewave:vffirewave/cppcheck:vfCopy head branch name to clipboard

Conversation

firewave commented Sep 29, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pfultz2 Oct 4, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

firewave Oct 4, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pfultz2 commented Oct 4, 2022

Uh oh!

firewave commented Oct 4, 2022

Uh oh!

firewave commented Aug 8, 2023

Uh oh!

firewave commented Apr 20, 2024

Uh oh!

firewave commented Oct 31, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

added (optional) lazy execution of ValueFlow#4521
firewave wants to merge 4 commits into
cppcheck-opensource:mainfrom
firewave:vf

pfultz2 Oct 4, 2022 •
edited
Loading

firewave Oct 4, 2022 •
edited
Loading