2

I was creating a vector of my bitpacked vectors, called xor_funcs, using the length and value constructor for vector. This is the test that failed:

TEST(vectorInit, size3) {
    const xor_func temp{false, {0,0,0}};
    vector<xor_func> arr{3, temp};
    for(xor_func& f : arr) {
        EXPECT_EQ(3, f.size()) << f;
    }
    for(int i = 0; i < 3; i++) {
        ASSERT_EQ(3, arr[i].size()) << "for i=" << i;
        arr[i].set(i);
    }
}

It seems that the size() call is accessing uninitialized memory, for vectors of length 3 or more, but not ones of size 2. Valgrind confirms that the memory is not recently stack'd, malloc'd or free'd.

The xor_func is defined as such:

class xor_func {
    private:
        boost::dynamic_bitset<> bitset;
        bool negated;
    public:
        xor_func(const bool neg, const std::initializer_list<int> lst); 
        // That's defined in cpp

        xor_func(const size_t size) : bitset(size), negated(false) {}

        // Disallow the trivial constructor, since I don't want
        // any 0 length xor_funcs existing by default.
        // xor_func() : bitset(), negated(false) {}

        xor_func(const bool negated, const boost::dynamic_bitset<> bitset)
            : bitset(bitset), negated(negated) {}
        // That's all the constructors.

        // Snip
}

I didn't do anything with the default copy and move constructors.

What is going on, and why does my test fail?

Theo Belaire
  • 2,980
  • 2
  • 22
  • 33
  • 1
    Well, something is probably broken in either `xor_func::xor_func(const bool neg, const std::initializer_list lst)` or `xor_func::size()`, since that's the two functions called in your test code...and you didn't give us the definition of either! – T.C. Jun 11 '14 at 21:48
  • The test that fails seems to be all about check `f.size()` where `f` is an `xor_func`. Yet you don't show `xor_func::size()`. Thus any answer must apparently be based on pure guesswork. – Cheers and hth. - Alf Jun 11 '14 at 21:54
  • 4
    `vector arr{3, temp};` You should rather use parentheses `(3, temp)` when you want to have a vector of three elements. Braces `{3, temp}` can be misinterpreted as multiple elements. One of the unfortunate properties of *uniform initialization*. (I think, it is, in this case, since `3` can be converted to `xor_func`) – dyp Jun 11 '14 at 21:56
  • 3
    You should make the `xor_func(size_t)` ctor `explicit` to avoid similar bugs. – dyp Jun 11 '14 at 21:58
  • Ah, that seems to explain what's going on. Let me just adjust my code and test it. – Theo Belaire Jun 11 '14 at 22:00
  • dyp: Can you post your deduction as an answer so I can accept? – Theo Belaire Jun 11 '14 at 22:02
  • :) of course, if that solved your problem.. but I'll (or someone else will) probably find a duplicate question instead.. – dyp Jun 11 '14 at 22:02
  • See also: http://stackoverflow.com/q/22501368/420683 – dyp Jun 11 '14 at 22:06
  • A more helpful answer is here I think: http://stackoverflow.com/questions/121162/what-does-the-explicit-keyword-in-c-mean, but of course, you need to know that the explicit call is needed. So while I agree that the information is included twice, I don't think it's a duplicate any more than a `map` is a `map`. However, my title is not the best overall anyways, so I'm fine with closing as a dup. – Theo Belaire Jun 11 '14 at 22:09
  • Closing as a dup is mainly to streamline attention to a single question. Duplicates are kept so that that central Q/A is found even with different wording, or, in your case, a related issue. I don't think your question is bad, and I agree it's far from obvious that it has something to do with `{}` vs `()`. But my answer would essentially be the same as for the other question. – dyp Jun 11 '14 at 22:22
  • Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/55468/discussion-between-tyr-and-dyp). – Theo Belaire Jun 11 '14 at 22:23

1 Answers1

5

As dyb said, vector<xor_func> arr{3, temp}; was being interpreted as vector<xor_func> arr({xor_func{3}, temp}), as the 3 could be converted into a xor_func by a constructor implicitly, and then it could choose the initializer list version of the constructor to call.

If you look at Is C++11 Uniform Initialization a replacement for the old style syntax?, you can see that one of the downsides of uniform initialization syntax is exactly this bug. A more trivial example is

// std::string constructor prototype for reference
// fill (6) 
string (size_t n, char c);

void main() {
    string myString{65, 'B'};
    cout << myString << endl;
}

This will print out "AB", instead of "BBBBBB...BBB", as it can convert 65 to 'A', and then it's as if we wrote myString{'A', 'B'}. To fix it, simply don't try and use the uniform initialization syntax for this call and replace it with

string myString(65, 'B');

Another way I could fix this bug is to change the constructor xor_func(const size_t size) to be explicit xor_func(const size_t size), which prevents the compiler from implicitly converting the 3 to an xor_func.

Oh, there's another great answer What does the explicit keyword mean in C++.

Community
  • 1
  • 1
Theo Belaire
  • 2,980
  • 2
  • 22
  • 33