0

I’m really new to c and c++

I tried to experiment with structs to create a list-like structure that can contain float and lists, basically.

This code compiles, but it behaves differently depending on the compiler:

  • with the latest version of visual studio community, it outputs 5 and then 0.

  • with an online shell, I get 5 and then 5

The one I would want to get is the second one when the vector gets passed through the function.

here is the code:

#include <iostream>
#include <vector>

using namespace std;

struct Box {
    Box(char t) {
        type = t;
    }
    union Value {
        float number;
        vector<Box>* elements;
    };
    Value value;
    char type;
};

Box newBox() {
    Box aBox('l');

    vector<Box> newVec;
    newVec.assign(5, Box('n'));


    aBox.value.elements = &newVec;
    cout << aBox.value.elements->size() << "\n";
    return aBox;

}

int main()
{
    Box test = newBox();
    cout << test.value.elements->size();  // this is not always working nicely
}


Where does that come from?

Is there something wrong with my code?

And is there a better way to create this kind of structure?

sepp2k
  • 363,768
  • 54
  • 674
  • 675
rambi
  • 1,013
  • 1
  • 7
  • 25
  • "This code compiles, but he behaves differently depending on the compiler:" There's usually only two reasons for this: 1) a compiler bug or 2) [Undefined Behavior](https://stackoverflow.com/questions/32132574/does-undefined-behavior-really-permit-anything-to-happen). I'll have to look closer at your code to help figure out which is at fault here. –  Jun 11 '20 at 22:07
  • The lifetime of `newVec` ends when `newBox` returns. Dereferencing a pointer to it has undefined behaviour. – molbdnilo Jun 11 '20 at 22:07
  • @Chipster -- or unspecified behavior: valid code whose meaning is determined by the implementation; or implementation-defined behavior: valid code whose meaning is determined by the implementation and documented by the implementation. – Pete Becker Jun 12 '20 at 12:59

2 Answers2

5

On this line:

aBox.value.elements = &newVec;

you are storing the address of a local variable. When you return from the newBox function, that variables dies, and then accessing that memory via the pointer invokes undefined behavior.

cigien
  • 57,834
  • 11
  • 73
  • 112
4

Is there something wrong with my code?

@cigien's answer explains what's wrong.

And is there a better way to create this kind of structure?

  1. Not related to your bug, but - don't use using namespace std.
  2. Avoid using raw pointers when you don't really need them; and certainly don't use them to transfer ownership.
  3. Use vocabulary types such as std::variant and std::optional instead of raw unions.

So, Try this:

#include <iostream>
#include <variant>
#include <vector>

struct Box {
    struct Item { float value; };
    using Boxes = std::vector<Box>;
    struct Empty {};

    Box(char t) : type(t) { }

    auto& item() &            { return std::get<Item>(contents); }
    const auto& item() const  { return std::get<Item>(contents); }

    auto& boxes()             { return std::get<Boxes>(contents); }
    const auto& boxes() const { return std::get<Boxes>(contents); }

    std::variant<Empty, Item, Boxes> contents { Empty{} };
    char type;
};

Box newBox() {
    Box aBox('l');

    Box::Boxes subBoxes;
    subBoxes.assign(5, Box('n'));

    aBox.contents = std::move(subBoxes);
    std::cout << "aBox has " << test.boxes().size() << " sub-boxes.\n";
    return aBox;
}

int main()
{
    Box test = newBox();
    std::cout << test.boxes().size() << '\n';
}

This is better - no pointers, and it's clearer what you can put in a box. However, note that making assumptions about the current active kind of element ina variant is usually a bad idea, so try to avoid std::get() in favor of std:visit() usually, to be on the safe side. Also, perhaps a method telling you how many items there in the box (in the shallow and the deep sense) could make sense. Read up on std::variant if you want to go in that direction.

einpoklum
  • 118,144
  • 57
  • 340
  • 684