etl icon indicating copy to clipboard operation
etl copied to clipboard

etl::optional doesn't compile with deleted copy constructor

Open mrdimfox opened this issue 6 years ago • 14 comments

If I use etl::optional with class like this:

struct A {
    A(int some) : _some(some) {}
    A(const A&) = delete;
    A(A&&) = delete;
    A& operator=(const A&) = delete;

  private:
    int _some;
};

and it does not compile with this error:

[build] ../src/etl/include\etl/optional.h:146:48: error: call to deleted constructor of 'A'
[build]      ::new (storage.template get_address<T>()) T(value_);
...
[build] ... note: 'A' has been explicitly marked deleted here
[build]     A(const A&) = delete;
[build]     ^

But it works ok if I use std::optional.

Version of ETL is 14.29.3. Compiler is "arm-none-eabi-gcc.exe (GNU Tools for Arm Embedded Processors 8-2019-q3-update) 8.3.1 20190703 (release) [gcc-8-branch revision 273027]".

mrdimfox avatar Sep 11 '19 12:09 mrdimfox

Can you check to see if std::optional works if you disable the assignment operator?

jwellbelove avatar Sep 12 '19 08:09 jwellbelove

Yes, it works, but only if you don't use the assignment. Seems logical.

mrdimfox avatar Sep 12 '19 09:09 mrdimfox

I'll have to figure out how std::optional constructs a new object from an existing one without using the copy constructor.

jwellbelove avatar Sep 12 '19 10:09 jwellbelove

I am curious in your example you have the move constructor for class A deleted as well. Is this accurate in your actual testing example?

mloehnig avatar Sep 13 '19 18:09 mloehnig

The C++17 STL version uses an rvalue reference value constructor, whilst the ETL's is C++03 style const reference. I shall add a conditional compile flag to choose an rvalue reference constructor if >= C++11.

jwellbelove avatar Sep 13 '19 19:09 jwellbelove

@swagggpickle, i have something like this:

#include <optional>

struct A {
    A(int some) : _some(some) {}
    A(const A&) = delete;
    A(A&&) = delete;
    A& operator=(const A&) = delete;

  private:
    std::optional<int> _some;
};

struct B {
    B(int a_val) : a(a_val) {}
    B() : a(std::nullopt) {}

    std::optional<A> a;
};

int main() {
    B b_with_some(1);
    B b_without_some;

    return 0;
}

Here is godbolt: https://godbolt.org/z/-XvjYM

mrdimfox avatar Sep 13 '19 19:09 mrdimfox

I've had a look at the code, but I still haven't bveen able to figure out yet how std::optional can create a non-trivially constructable object that has a deleted copy constructor, move constructor and assignment operator!

jwellbelove avatar Sep 16 '19 10:09 jwellbelove

Any chance the code for the struct is optimized out because b_with_some and b_without_some are never used? I noticed -O1 as a compiler option on godbolt.

annius avatar Sep 23 '19 21:09 annius

@annius Nope. You can remove it, the flag, and see the same picture: it is compiled successfully.

mrdimfox avatar Sep 23 '19 21:09 mrdimfox

@mrdimfox I mentioned this thread to a colleague and maybe he'll chime in himself, but he pointed out that if you look at the generated code on godbolt, there is nothing actually generated for any of the struct methods except for the constructor (even without optimization).

If your code is modified to actually use one of the deleted constructors (instead of simply instantiating the object), it will fail to compile.

For example, if you change the last-but-one line to

B b_without_some = without_some;

annius avatar Dec 19 '19 22:12 annius

If you read the documentation for std::optional, I think that the issue may be down to the conditional explicitness of the various std::optional constructors.

jwellbelove avatar Dec 20 '19 10:12 jwellbelove

@annius You are right, but problem is here: if I try to compile my code above with etl instead of std it does not compile. If I use std it does. I don't want to use a copy constructor explicitly like you did in the example. I want exact my code from the snippet above to be compiled.

Compiles:

#include <optional>

struct A {
    A(int some) : _some(some) {}
    A(const A&) = delete;
    A(A&&) = delete;
    A& operator=(const A&) = delete;

  private:
    std::optional<int> _some;
};

struct B {
    B(int a_val) : a(a_val) {}
    B() : a(std::nullopt) {}

    std::optional<A> a;
};

int main() {
    B b_with_some(1);
    B b_without_some;

    return 0;
}

Does not compile:

#include <etl/optional.h>

struct A {
    A(int some) : _some(some) {}
    A(const A&) = delete;
    A(A&&) = delete;
    A& operator=(const A&) = delete;

  private:
    etl::optional<int> _some;
};

struct B {
    B(int a_val) : a(a_val) {}
    B() : a(etl::nullopt) {}

    etl::optional<A> a;
};

int main() {
    B b_with_some(1);
    B b_without_some;

    return 0;
}

mrdimfox avatar Dec 20 '19 15:12 mrdimfox

The odd thing is that the compiler is trying to call A(const A&) when compiling B(int a_val) : a(a_val) {} I don't see why it should be calling the copy constructor for A when constructing with an int.

jwellbelove avatar Dec 21 '19 10:12 jwellbelove

It's a long time since I looked at this, but I now think that the etl::optional copy & move constructors need to have conditional overloads that memcpy values that are trivially copy constructible. If I make the changes then this will only work correctly if either the compiler supports the common traits built-ins or the user defines the overloaded traits for each type.

jwellbelove avatar Sep 04 '21 17:09 jwellbelove