Skip to content

Conversation

@chrchr-github
Copy link
Collaborator

@chrchr-github chrchr-github commented Nov 19, 2024

Co-authored-by: Paul Fultz II [email protected]

lib/checkstl.cpp Outdated
}
if (values.size() == 1)
return values.front();
it = std::find_if(values.cbegin(), values.cend(), [](const ValueFlow::Value& v) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is problematic if there is divergent lifetimes, that is why we check the size is 1 or no duplicates for iterator kinds. Why is there multiple lifetimes? Do we have a lifetime for t.s.a and a both?

Copy link
Collaborator Author

@chrchr-github chrchr-github Dec 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is problematic if there is divergent lifetimes, that is why we check the size is 1 or no duplicates for iterator kinds. Why is there multiple lifetimes? Do we have a lifetime for t.s.a and a both?

In the s.a example, the lifetime tokvalues are s and .. But I think it makes sense to choose the dot, unless there can be multiple dots.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its the . in s.a I assume? Ideally we should remove the lifetimes that refer to a sub expression of another lifetime from the vector. Something like this function could be called here(its not tested though):

std::vector<ValueFlow::Value> pruneLifetimes(std::vector<ValueFlow::Value> lifetimes)
{
    std::vector<ValueFlow::Value> result;
    auto start = lifetimes.begin();
    while(start != lifetimes.end())
    {
        const Token* tok1 = start->tokvalue;
        auto it = std::partition(start, lifetimes.end(), [&](const ValueFlow::Value& v) { 
            const Token* tok2 = v.tokvalue;
            return astHasToken(tok1, tok2) or astHasToken(tok2, tok1); 
        });
        auto root = std::min_element(start, it, [](const ValueFlow::Value& x, const ValueFlow::Value& y) {
            return astHasToken(x.tokvalue, y.tokvalue);
        });
        result.push_back(*root);
        start = it;
    }
    return result;
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not entirely sure what it does, but it seems to work 👍

Copy link
Contributor

@pfultz2 pfultz2 Dec 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First it groups the lifetimes together using std::partition with the lifetimes that refer to the same token or token of a subexpression. Then it finds the lifetime in that group that refers to the "highest" parent using std::min_element and adds that to the vector. Hopefully that makes sense.

@chrchr-github chrchr-github merged commit 9d64da9 into danmar:main Dec 8, 2024
60 checks passed
@chrchr-github chrchr-github deleted the chr_13327 branch December 8, 2024 21:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants