Inconsistent output with std::any_of

Question

I have two version of the same function using std::any_of and std::find_if for checking whether an int exist in a vector (returned from g.getEdges()).

However when I repeatedly run the std::any_of the return value is inconsistent, while std::find_if is.

Both versions are as follow (g.getEdges() returns an int vector, while this->s is an int):

bool Search::marked(const int &v) {
    return std::any_of(begin(g.getEdges(v)), end(g.getEdges(v)), [this](const int &w) { return w == this->s; });
}

bool Search::marked(const int &v) {
    auto it = std::find_if(begin(g.getEdges(v)), end(g.getEdges(v)), [this](const auto &w) { return w == this->s; });
    return it != end(g.getEdges(v));
}

In both classes, adj is the vector being iterated:

class Graph {
  public:
    Graph() : vertex(0), edges(0) {
      adj = std::make_unique<std::vector<std::vector<int>>>(0);
    }
    const std::vector<int> getEdges(int v) { return (*adj)[v]; }
  // ... rest of public
  private:
    std::shared_ptr<std::vector<std::vector<int>>> adj; // Shared for Search class
}

class Search {
  public:
    Search(Graph &g_, int s_) : g(g_), s(s_) {}
    int count();
    bool marked(const int &v);

  private:
    Graph &g;
    int s;
};

The gtest function I used to check the code:

TEST(searchClassTest, HandlesPositiveInput) {
    // ... init myGraph ...
    Search mySearch(myGraph, 2);  // 2 is s!
    EXPECT_EQ(mySearch.count(), 2);
    EXPECT_EQ(mySearch.marked(1), true);
    EXPECT_EQ(mySearch.marked(3), true);
}

Why am I getting inconsistent results?

Link to minimal reproducible example: https://godbolt.org/z/9TnGq3Wbs

Please make an minimal reproducible example. In this case, post an example input for which you get inconsistent results, and sufficient code so that we can reproduce the issue. — cigien
– cigien, Commented Apr 10 at 10:31
OT: return condition ? true : false; is the complicated way to say return condition. — 463035818_is_not_an_ai
– 463035818_is_not_an_ai, Commented Apr 10 at 10:32
Please don't describe code. Instead of "g.getEdges() returns an int vector" show the declaration - it isn't clear if getEdges returns a copy of the vector or a reference. — Yksisarvinen
– Yksisarvinen, Commented Apr 10 at 10:33
You call begin and end on two different copies of the vector. This cannot work. Call getEdges only once, and then operator on that copy. — j6t
– j6t, Commented Apr 10 at 10:50
BTW, it is an antipattern to return by const value. Do not do that. The most obvious disadvantage is that it is impossible to "move from" such a return value. — j6t
– j6t, Commented Apr 10 at 10:52

Dominik Kaszewski · Accepted Answer · 2025-04-10 10:52:23Z

8

The code you presented is totally invalid due to

const std::vector<int> getEdges(int v)

In both implementation of marked, you call this function multiple times, each time getting a different copy of a vector, then try to compare their iterators:

bool Search::marked(const int &v) {
    // begin and end point to two different vectors
    auto it = std::find_if(begin(g.getEdges(v)), end(g.getEdges(v)), [this](const auto &w) { return w == this->s; });
    // this end points to a third one
    return it != end(g.getEdges(v)) ? true : false;
}

The std::any_of version is broken in the same way. Trying to iterate from one vector to another is Undefined Behavior, so any results you see might as well be random - function might return true, false, 7 or "nasal demons, here I come".

Simply changing getEdges to return a reference should fix this.

answered Apr 10 at 10:52

Dominik Kaszewski

3,1441 gold badge16 silver badges31 bronze badges

Sign up to request clarification or add additional context in comments.

3 Comments

Yat Fung Apr 10 at 11:00

Pass by reference fixed the issue, thank you!

Dominik Kaszewski Apr 10 at 11:05

@YatFung Glad I could help. Both references and iterators (in the case of a vector) are just glorified pointers, so you might want to study them before diving deep into algorithms, otherwise you might run into hard to diagnose lifetime issues similar to this.

Jesper Juhl Apr 10 at 12:10

Another option (if one doesn't want to change getEdges()) would be to just call g.getEdges(v) once and then operate on the returned copy, like auto edges = g.getEdges(v); and then do everything on the edges vector. Not saying that's the best option, just pointing out that it is a possibility.

Collectives™ on Stack Overflow

Inconsistent output with std::any_of

1 Answer 1

3 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

3 Comments

Your Answer

Sign up or log in

Post as a guest

Related