7

I can see that that classes are treated as complex objects which are required for calling default constructor:

void QVector<T>::defaultConstruct(T *from, T *to)
{
    if (QTypeInfo<T>::isComplex) {
        while (from != to) {
            new (from++) T();
        }
    ...
}

But it's not clear why is it needed to construct objects in the 'hidden' area of QVector. I mean these objects are not accessible at all, so why not just to reserve the memory instead of the real object creation?

And as a bonus question, I would like to ask, if I want to have an array of non-default-constractible objects, can I safely replace QVector<T> with QVector<Wrapper<T>? where Wrapper is something like that:

class Wrapper {
public:
    union {
        T object;
        bool hack;
    };
    Wrapper() {}
    Wrapper(const T &t) : object { t }  {}
    Wrapper(const Wrapper &t) : object { t.object } {}

    Wrapper &operator=(const Wrapper &value) {
        object = value.object;
        return *this;
    }

    ~Wrapper() {}
};
demonplus
  • 5,613
  • 12
  • 49
  • 68
Grief
  • 1,839
  • 1
  • 21
  • 40
  • If an object does not have a default constructor, it is usually for a reason. I'd advise against hacking. Maybe try storing `std::unique_ptr` in your wrapper. – Neil Kirk Oct 27 '15 at 23:42
  • 4
    The default constructor is used by `QVector(int)` and `resize(int)` for elements that are actually in the vector. – aschepler Oct 27 '15 at 23:46
  • 1
    @NeilKirk I don't see any advantages in usage of `std::unique_ptr` over this hack, while there is a huge disadvantage in grinding the memory which will bring to naught the advantages of `QVector` itself. Also, please don't forget, that `std::vector` doesn't have a requirement for having a default constructor. – Grief Oct 27 '15 at 23:51
  • @aschepler good point! But I don't need this functionality. Even if I would need it, I can use `QVector(int size, const T & value)` instead, right? Are there any other issues with my approach? – Grief Oct 27 '15 at 23:54
  • An advantage would be that it isn't using undefined behavior. You could use placement new instead. – Neil Kirk Oct 27 '15 at 23:54
  • @NeilKirk where did you find an UB in my code? Placement new won't eradicate memory granularity, all elements would be located at random addresses in memory while `QVector` is designed for linear memory allocation for all its elements. – Grief Oct 27 '15 at 23:57
  • 1
    No, placement new lets you put your object in a buffer of your choosing. It could be a member of your wrapper. If you try to use your object - including copying a new object into it - when it was created using the "default constructor" thanks to your hack, it is undefined behavior. – Neil Kirk Oct 27 '15 at 23:59
  • @NeilKirk Oh, you are definetely correct. I missed that the constructor won't be called before assignment at all. My fault. But I am not sure how to mix placement new with `unique_ptr`, could you please help me with the code for the `Wrapper` as an answer here? – Grief Oct 28 '15 at 00:12
  • I haven't used placement new much so I can't give a concrete example. Look up `std::aligned_storage` and add a member of it to your wrapper, and use placement new when you want to "active" the object. You need to be very careful your object is created at the right time and destroyed properly! When copying, is the target object alive yet or not? – Neil Kirk Oct 28 '15 at 00:14
  • 1
    `T QVector::value(int i) const` also needs a default constructor because if index is out of bounds, you get a default value. – ymoreau Nov 17 '17 at 10:13
  • Ouch, this is a really unfortunate over-constrainment. – Lightness Races in Orbit Jan 14 '19 at 12:23
  • Possible duplicate of [No matching call to default constructor, when using QVector](https://stackoverflow.com/questions/54181249/no-matching-call-to-default-constructor-when-using-qvector) – ymoreau Jan 29 '19 at 12:35

1 Answers1

0

It's easy enough to make the QVector work for a non-default-constructible type T:

#define QVECTOR_NON_DEFAULT_CONSTRUCTIBLE(Type) \
template <> QVector<Type>::QVector(int) = delete; \
template <> void QVector<Type>::resize(int newSize) { \
   Q_ASSERT(newSize <= size()); \
   detach(); \
} \
template <> void QVector<Type>::defaultConstruct(Type*, Type*) { Q_ASSERT(false); }

The macro needs to be present right after MyType declaration - in the header file (if any), and it must be in namespace or global scope:

struct MyType { ... };
QVECTOR_NON_DEFAULT_CONSTRUCTIBLE(MyType)

struct A {
  struct MyType2 { ... };
};
QVECTOR_NON_DEFAULT_CONSTRUCTIBLE(A::MyType2);

No, the wrapper is not correct. It doesn't destruct the object member. It also doesn't offer move semantics, doesn't protect from being default-constructed, etc. The hack union member is not necessary. Nothing in a union will be default-constructed for you.

Here's a more correct wrapper - it pretty much resembles std::optional. See here to see how much nuance an optional needs :)

// https://github.com/KubaO/stackoverflown/tree/master/questions/vector-nodefault-33380402

template <typename T> class Wrapper final {
   union {
      T object;
   };
   bool no_object = false;
   void cond_destruct() {
      if (!no_object)
         object.~T();
      no_object = true;
   }
public:
   Wrapper() : no_object(true) {}
   Wrapper(const Wrapper &o) : no_object(o.no_object) {
      if (!no_object)
         new (&object) T(o.object);
   }
   Wrapper(Wrapper &&o) : no_object(o.no_object) {
      if (!no_object)
         new (&object) T(std::move(o.object));
   }
   Wrapper(const T &o) : object(o) {}
   Wrapper(T &&o) : object(std::move(o)) {}
   template <class...Args> Wrapper(Args...args) : object(std::forward<Args>(args)...) {}
   template <class U, class...Args> Wrapper(std::initializer_list<U> init, Args...args) :
      object(init, std::forward<Args>(args)...) {}
   operator T&      () &      { assert(!no_object); return object; }
   operator T&&     () &&     { assert(!no_object); return std::move(object); }
   operator T const&() const& { assert(!no_object); return object; }
   Wrapper &operator=(const Wrapper &o) & {
      if (no_object)
         ::new (&object) T(o);
      else
         object = o.object;
      no_object = false;
      return *this;
   }
   Wrapper &operator=(Wrapper &&o) & {
      if (no_object)
         ::new (&object) T(std::move(o.object));
      else
         object = std::move(o.object);
      no_object = false;
      return *this;
   }
   template<class... Args> T &emplace(Args&&... args) {
      cond_destruct();
      ::new (&object) T(std::forward<Args>(args)...);
      no_object = false;
      return object;
   }
   ~Wrapper() {
      cond_destruct();
   }
};

Since the assignment operators are ref-qualified, it disallows assigning to rvalues, so it has the IMHO positive property that the following won't compile:

Wrapper<int>() = 1   // likely Wrapper<int>() == 1 was intended
Kuba hasn't forgotten Monica
  • 95,931
  • 16
  • 151
  • 313
  • `detach()` ensures that the container doesn't share data with any other instance: if it did, a deep copy is made. – Kuba hasn't forgotten Monica Sep 10 '18 at 16:26
  • But then if I understood it correctly, the `resize` function won't actually resize anything. That would also mean that `clear()` won't clear the container, since if you look at the source code, `clear()` calls `resize(0)`. – Donald Duck Sep 10 '18 at 17:09
  • That’s correct. As far as I can remember now, it is a limitation necessary to have `QVector` support those types: it doesn’t leave all functionality intact. It’s a shortcoming of the design of this class. I imagine that sometime soon, Qt’s Containers will become deprecated and retained for backwards compatibility — I may be wrong, but changes to extend their functionality are not accepted — the goal being not to back the API into some unwieldy corner. It’d be worth checking whether some firm roadmap decisions were made in that regard. – Kuba hasn't forgotten Monica Sep 11 '18 at 12:30