[Bug c++/110905] New: GCC rejects constexpr code that may re-initialize union member

[Bug c++/110905] New: GCC rejects constexpr code that may re-initialize union member

[danakj at orodu dot net]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110905

            Bug ID: 110905
           Summary: GCC rejects constexpr code that may re-initialize
                    union member
           Product: gcc
           Version: 13.2.1
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: danakj at orodu dot net
  Target Milestone: ---

Godbolt: https://gcc.godbolt.org/z/v5anxqnP1

This repro contains a std::optional (which has a union) and it sets the union
in a loop. Doing so causes GCC to reject the code as not being a constant
expression. The error I was getting in my project was far more descriptive,
with it trying to call the deleted constructor of the union.

error: use of deleted function
‘sus::option::__private::Storage<sus::containers::VecIntoIter<sus::num::i32>,
false>::<unnamed union>::<constructor>()’

In my more minimal test case the error is more terse and less clear.

<source>:62:59: error: non-constant condition for static assertion
   62 | static_assert(Flatten<int>({{1, 2, 3}, {}, {4, 5}}).sum() == 1 + 2 + 3
+ 4 + 5);
      |              
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~
<source>:62:51: error: '(((const std::vector<int, std::allocator<int>
>*)(&<anonymous>)) != 0)' is not a constant expression
   62 | static_assert(Flatten<int>({{1, 2, 3}, {}, {4, 5}}).sum() == 1 + 2 + 3
+ 4 + 5);
      |           

```cpp
#include <optional>
#include <vector>

template <class T>
struct VectorIter {
    constexpr std::optional<T> next() {
        if (front == back) return std::optional<T>();
        T& item = v[front];
        front += 1u;
        return std::optional<T>(std::move(item));
    }

    constexpr VectorIter(std::vector<T> v2) : v(std::move(v2)), front(0u),
back(v.size()) {}
    VectorIter(VectorIter&&) = default;
    VectorIter& operator=(VectorIter&&) = default;

    std::vector<T> v;
    size_t front;
    size_t back;
};

template <class T>
struct Flatten {
    constexpr Flatten(std::vector<std::vector<T>> v) : vec(std::move(v)) {}

    constexpr std::optional<T> next() {
        std::optional<T> out;
        while (true) {
            // Take an item off front_iter_ if possible.
            if (front_iter_.has_value()) {
                out = front_iter_.value().next();
                if (out.has_value()) return out;
                front_iter_ = std::nullopt;
            }
            // Otherwise grab the next vector into front_iter_.
            if (!vec.empty()) {
                std::vector<T> v = std::move(vec[0]);
                vec.erase(vec.begin());
                front_iter_.emplace([](auto&& iter) {
                    return VectorIter<T>(std::move(iter));
                }(std::move(v)));
            }
            if (!front_iter_.has_value()) break;
        }
        return out;
    }

    constexpr T sum() && {
        T out = T();
        while (true) {
            std::optional<T> i = next();
            if (!i.has_value()) break;
            out += *i;
        }
        return out;
    }

    std::vector<std::vector<T>> vec;
    std::optional<VectorIter<T>> front_iter_;
};

static_assert(Flatten<int>({{1, 2, 3}, {}, {4, 5}}).sum() == 1 + 2 + 3 + 4 +
5);

int main() {}

```

When the Flatten::next() method is simplified a bit, so that it can see the
union is only initialized once, the GCC compiler no longer rejects the code.
https://gcc.godbolt.org/z/szfGsdxb7

```cpp
#include <optional>
#include <vector>

template <class T>
struct VectorIter {
    constexpr std::optional<T> next() {
        if (front == back) return std::optional<T>();
        T& item = v[front];
        front += 1u;
        return std::optional<T>(std::move(item));
    }

    constexpr VectorIter(std::vector<T> v2) : v(std::move(v2)), front(0u),
back(v.size()) {}
    VectorIter(VectorIter&&) = default;
    VectorIter& operator=(VectorIter&&) = default;

    std::vector<T> v;
    size_t front;
    size_t back;
};

template <class T>
struct Flatten {
    constexpr Flatten(std::vector<T> v) : vec(std::move(v)) {}

    constexpr std::optional<T> next() {
        std::optional<T> out;
        while (true) {
            // Take an item off front_iter_ if possible.
            if (front_iter_.has_value()) {
                out = front_iter_.value().next();
                if (out.has_value()) return out;
                front_iter_ = std::nullopt;
            }
            // Otherwise grab the next vector into front_iter_.
            if (!moved) {
                std::vector<T> v = std::move(vec);
                moved = true;
                front_iter_.emplace([](auto&& iter) {
                    return VectorIter<T>(std::move(iter));
                }(std::move(v)));
            }
            if (!front_iter_.has_value()) break;
        }
        return out;
    }

    constexpr T sum() && {
        T out = T();
        while (true) {
            std::optional<T> i = next();
            if (!i.has_value()) break;
            out += *i;
        }
        return out;
    }

    bool moved = false;
    std::vector<T> vec;
    std::optional<VectorIter<T>> front_iter_;
};

static_assert(Flatten<int>({1, 2, 3}).sum() == 1 + 2 + 3);

int main() {}
```

Yet in the first example, the GCC compiler still rejects the code if only a
single vector is passed in, so that the union is only initialized once, in the
same way as the 2nd example:

```
static_assert(Flatten<int>({{1, 2, 3}}).sum() == 1 + 2 + 3);
```

[Bug c++/110905] GCC rejects constexpr code that may re-initialize union member

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110905

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2023-08-04
             Status|UNCONFIRMED                 |WAITING
     Ever confirmed|0                           |1

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
>In my more minimal test case the error is more terse and less clear.


The reduced testcase is a different issue and is a dup of bug 85944.

In the first testcase provided below if we move the static_assert into main
instead of the toplevel, it gets accepted.

I think you need to redo your reduction.

[Bug c++/110905] GCC rejects constexpr code that may re-initialize union member

[danakj at orodu dot net]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110905

--- Comment #2 from danakj at orodu dot net ---
Ah ok. Here's a big reproduction: https://godbolt.org/z/Kj7Tcd6P4

/opt/compiler-explorer/gcc-trunk-20230804/include/c++/14.0.0/bits/stl_construct.h:97:14:
  in 'constexpr' expansion of
'((sus::containers::VecIntoIter<sus::num::i32>*)<anonymous>)->sus::containers::VecIntoIter<sus::num::i32>::VecIntoIter((*
& std::forward<sus::containers::VecIntoIter<sus::num::i32> >((* & __args#0))))'
<source>:32895:22: error: use of deleted function
'sus::option::__private::Storage<sus::containers::VecIntoIter<sus::num::i32>,
false>::<unnamed union>::<constructor>()'
32895 | struct [[nodiscard]] VecIntoIter final
      |                      ^~~~~~~~~~~
<source>:3015:9: note:
'sus::option::__private::Storage<sus::containers::VecIntoIter<sus::num::i32>,
false>::<unnamed union>::<constructor>()' is implicitly deleted because the
default definition would be ill-formed:
 3015 |   union {
      |         ^
<source>:3015:9: error: no matching function for call to
'sus::containers::VecIntoIter<sus::num::i32>::VecIntoIter()'
<source>:32953:13: note: candidate: 'constexpr
sus::containers::VecIntoIter<ItemT>::VecIntoIter(sus::containers::Vec<ItemT>&&,
sus::num::usize, sus::num::usize) [with ItemT = sus::num::i32]'
32953 |   constexpr VecIntoIter(Vec<Item>&& vec, usize front, usize back)
noexcept
      |             ^~~~~~~~~~~
<source>:32953:13: note:   candidate expects 3 arguments, 0 provided
<source>:32951:13: note: candidate: 'constexpr
sus::containers::VecIntoIter<ItemT>::VecIntoIter(sus::containers::Vec<ItemT>&&)
[with ItemT = sus::num::i32]'
32951 |   constexpr VecIntoIter(Vec<Item>&& vec) noexcept :
vec_(::sus::move(vec)) {}
      |             ^~~~~~~~~~~
<source>:32951:13: note:   candidate expects 1 argument, 0 provided
<source>:32895:22: note: candidate: 'constexpr
sus::containers::VecIntoIter<sus::num::i32>::VecIntoIter(sus::containers::VecIntoIter<sus::num::i32>&&)'
32895 | struct [[nodiscard]] VecIntoIter final
      |                      ^~~~~~~~~~~
<source>:32895:22: note:   candidate expects 1 argument, 0 provided
Compiler returned: 1


I will try to shrink it now.

[Bug c++/110905] GCC rejects constexpr code that may re-initialize union member

[danakj at orodu dot net]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110905

--- Comment #3 from danakj at orodu dot net ---
Repro down from 37k to under 1000 lines now: https://godbolt.org/z/enMxaqjb6

[Bug c++/110905] GCC rejects constexpr code that may re-initialize union member

[danakj at orodu dot net]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110905

--- Comment #4 from danakj at orodu dot net ---
Ok it only happens if the VecIntoIter class has a base class, even when it's
empty the bug reproduces. But when I remove the IteratorBase base class, then
GCC compiles it correctly.

It's getting hard to remove anything and keep it to repro at this point.
Replacing Option with std::optional also made it stop, for whatever reason.

https://godbolt.org/z/a9PcsKTMf

```
#include <concepts>
#include <cstddef>
#include <memory>
#include <utility>

template <class T>
class Vec;

constexpr int from_sum(auto&& it) noexcept {
    auto p = int(0);
    while (true) {
        auto i = it.next();
        if (!i.has_value()) break;
        p += *i;
    }
    return p;
}

template <class T>
struct Storage final {
    constexpr ~Storage()
        requires(std::is_trivially_destructible_v<T>)
    = default;
    constexpr ~Storage()
        requires(!std::is_trivially_destructible_v<T>)
    {}

    constexpr Storage(const Storage&)
        requires(std::is_trivially_copy_constructible_v<T>)
    = default;
    constexpr Storage& operator=(const Storage&)
        requires(std::is_trivially_copy_assignable_v<T>)
    = default;
    constexpr Storage(Storage&&)
        requires(std::is_trivially_move_constructible_v<T>)
    = default;
    constexpr Storage& operator=(Storage&&)
        requires(std::is_trivially_move_assignable_v<T>)
    = default;

    constexpr Storage() {}
    constexpr Storage(const std::remove_cvref_t<T>& t)
        : val_(t), state_(true) {}
    constexpr Storage(std::remove_cvref_t<T>& t) : val_(t), state_(true) {}
    constexpr Storage(std::remove_cvref_t<T>&& t)
        : val_(std::move(t)), state_(true) {}

    __attribute__((pure)) constexpr const T& val() const { return val_; }
    __attribute__((pure)) constexpr T& val_mut() { return val_; }

    __attribute__((pure)) constexpr inline bool state() const noexcept {
        return state_;
    }

    constexpr inline void construct_from_none(const T& t) noexcept
        requires(std::is_copy_constructible_v<T>)
    {
        std::construct_at(&val_, t);
        state_ = true;
    }
    constexpr inline void construct_from_none(T&& t) noexcept {
        std::construct_at(&val_, std::move(t));
        state_ = true;
    }

    constexpr inline void set_some(const T& t) noexcept
        requires(std::is_copy_constructible_v<T>)
    {
        if (state_ == false)
            construct_from_none(t);
        else
            val_ = t;
        state_ = true;
    }
    constexpr inline void set_some(T&& t) noexcept {
        if (state_ == false)
            construct_from_none(std::move(t));
        else
            val_ = std::move(t);
        state_ = true;
    }

    [[nodiscard]] constexpr inline T replace_some(T&& t) noexcept {
        return std::exchange(val_, std::move(t));
    }

    [[nodiscard]] constexpr inline T take_and_set_none() noexcept {
        state_ = false;
        auto taken = T(static_cast<T&&>(val_));
        val_.~T();
        return taken;
    }

    constexpr inline void set_none() noexcept {
        state_ = false;
        val_.~T();
    }

    constexpr inline void destroy() noexcept { val_.~T(); }

   private:
    union {
        T val_;
    };
    bool state_ = false;
};

template <class T>
class Option final {
    static_assert(!std::is_reference_v<T>);
    static_assert(!std::is_const_v<T>);

   public:
    inline constexpr Option() noexcept = default;

    static inline constexpr Option with(const T& t) noexcept
        requires(std::is_copy_constructible_v<T>)
    {
        return Option(t);
    }

    static inline constexpr Option with(T&& t) noexcept {
        if constexpr (std::is_move_constructible_v<T>) {
            return Option(std::move(t));
        } else {
            return Option(t);
        }
    }

    constexpr ~Option() noexcept
        requires(std::is_trivially_destructible_v<T>)
    = default;

    constexpr inline ~Option() noexcept
        requires(!std::is_trivially_destructible_v<T>)
    {
        if (t_.state()) t_.destroy();
    }

    constexpr Option(Option&& o)
        requires(std::is_trivially_move_constructible_v<T>)
    = default;

    constexpr Option(Option&& o) noexcept
        requires(!std::is_trivially_move_constructible_v<T>)
    {
        if (o.t_.state()) t_.construct_from_none(o.t_.take_and_set_none());
    }

    constexpr Option(Option&& o)
        requires(!std::is_move_constructible_v<T>)
    = delete;

    constexpr Option& operator=(Option&& o)
        requires(std::is_trivially_move_assignable_v<T>)
    = default;

    constexpr Option& operator=(Option&& o) noexcept
        requires(!std::is_trivially_move_assignable_v<T>)
    {
        if (o.t_.state())
            t_.set_some(o.t_.take_and_set_none());
        else if (t_.state())
            t_.set_none();
        return *this;
    }

    constexpr Option& operator=(Option&& o)
        requires(!std::is_move_constructible_v<T>)
    = delete;

    __attribute__((pure)) constexpr bool has_value() const noexcept {
        return t_.state();
    }

    __attribute__((pure)) constexpr const std::remove_reference_t<T>&
    operator*() const& noexcept {
        return t_.val();
    }
    __attribute__((pure)) constexpr std::remove_reference_t<T>&
    operator*() & noexcept {
        return t_.val_mut();
    }

   private:
    template <class U>
    friend class Option;

    constexpr explicit Option(const T& t) : t_(t) {}
    constexpr explicit Option(T&& t) : t_(std::move(t)) {}

    Storage<T> t_;
};

template <class Iter, class Item>
class IteratorBase {
   public:
};

template <class EachIter, class InnerSizedIter>
class [[nodiscard]] Flatten
    : public IteratorBase<Flatten<EachIter, InnerSizedIter>,
                          typename EachIter::Item> {
   public:
    using Item = typename EachIter::Item;

    constexpr Flatten(InnerSizedIter&& iters) : iters_(std::move(iters)) {}

    constexpr Option<Item> next() noexcept {
        Option<Item> out;
        while (true) {
            if (front_iter_.has_value()) {
                out = (*front_iter_).next();
                if (out.has_value()) return out;
                front_iter_ = Option<EachIter>();
            }

            front_iter_ = [](auto&& i) {
                if (!i.has_value()) return Option<EachIter>();
                return Option<EachIter>::with(std::move(*i).into_iter());
            }(iters_.next());
            if (!front_iter_.has_value()) break;
        }

        return out;
    }

    constexpr Item sum() && noexcept { return from_sum(std::move(*this)); }

   private:
    InnerSizedIter iters_;
    Option<EachIter> front_iter_;
};

template <class ItemT>
struct [[nodiscard]] VecIntoIter
    : public IteratorBase<VecIntoIter<ItemT>, ItemT> {
   public:
    using Item = ItemT;

    static constexpr auto with(Vec<Item>&& vec) noexcept {
        return VecIntoIter(std::move(vec));
    }

    constexpr Option<Item> next() noexcept {
        if (front_index_ == back_index_) [[unlikely]]
            return Option<Item>();

        Item& item =
            vec_[std::exchange(front_index_, front_index_ + size_t{1})];
        return Option<Item>::with(std::move(item));
    }

    constexpr auto flatten() && noexcept {
        using Flatten = Flatten<
            decltype(std::declval<std::remove_cvref_t<Item>&&>().into_iter()),
            VecIntoIter>;
        return Flatten(std::move(*this));
    }

   private:
    constexpr VecIntoIter(Vec<Item>&& vec) noexcept : vec_(std::move(vec)) {}

    Vec<Item> vec_;
    size_t front_index_ = size_t{0};
    size_t back_index_ = vec_.len();
};

template <class T>
class Vec final {
    static_assert(!std::is_reference_v<T>);
    static_assert(!std::is_const_v<T>);

   public:
    template <std::convertible_to<T>... Ts>
    static inline constexpr Vec with(Ts&&... values) noexcept {
        auto v = Vec(nullptr, size_t{0}, size_t{0});
        v.grow_to_exact(sizeof...(Ts));
        (..., v.push(std::forward<Ts>(values)));
        return v;
    }

    constexpr ~Vec() {
        if (is_alloced()) free_storage();
    }

    constexpr Vec(Vec&& o) noexcept
        : data_(std::exchange(o.data_, nullptr)),
          len_(std::exchange(o.len_, 0)),
          capacity_(std::exchange(o.capacity_, 0)) {}
    constexpr Vec& operator=(Vec&& o) noexcept {
        if (is_alloced()) free_storage();
        data_ = std::exchange(o.data_, nullptr);
        len_ = std::exchange(o.len_, 0);
        capacity_ = std::exchange(o.capacity_, 0);
        return *this;
    }

    constexpr void grow_to_exact(size_t cap) noexcept {
        if (cap <= capacity_) return;
        const auto bytes = sizeof(T) * cap;

        if (!is_alloced()) {
            data_ = std::allocator<T>().allocate(size_t{cap});
            capacity_ = cap;
            return;
        }

        T* new_allocation = std::allocator<T>().allocate(size_t{cap});
        T* old_t = data_;
        T* new_t = new_allocation;

        const size_t self_len = len();
        for (size_t i = 0; i < self_len; i += 1u) {
            std::construct_at(new_t, std::move(*old_t));
            old_t->~T();
            ++old_t;
            ++new_t;
        }
        std::allocator<T>().deallocate(data_, size_t{capacity_});
        data_ = new_allocation;
        capacity_ = cap;
    }

    constexpr void reserve(size_t additional) noexcept {
        if (len() + additional <= capacity_) return;
        grow_to_exact(apply_growth_function(additional));
    }

    constexpr void push(T t) noexcept
        requires(std::is_move_constructible_v<T>)
    {
        reserve(size_t{1});
        const auto self_len = len();
        std::construct_at(data_ + self_len, std::move(t));
        len_ = self_len + size_t{1};
    }

    constexpr VecIntoIter<T> into_iter() && noexcept
        requires(std::is_move_constructible_v<T>)
    {
        return VecIntoIter<T>::with(std::move(*this));
    }

    __attribute__((pure)) constexpr T& operator[](size_t i) & noexcept {
        return *(data_ + i);
    }

    __attribute__((pure)) constexpr inline size_t len() const& noexcept {
        return len_;
    }

   private:
    constexpr Vec(T* ptr, size_t len, size_t cap)
        : data_(ptr), len_(len), capacity_(cap) {}

    constexpr size_t apply_growth_function(size_t additional) const noexcept {
        size_t goal = additional + len();
        size_t cap = capacity_;

        while (cap < goal) {
            cap = (cap + 1u) * 3u;
            auto bytes = sizeof(T) * cap;
        }
        return cap;
    }

    constexpr void free_storage() {
        if constexpr (!std::is_trivially_destructible_v<T>) {
            const auto self_len = len();
            for (size_t i = 0; i < self_len; i += 1u) (data_ + i)->~T();
        }
        std::allocator<T>().deallocate(data_, size_t{capacity_});
    }

    constexpr inline bool is_alloced() const noexcept {
        return capacity_ > size_t{0};
    }

    T* data_;
    size_t len_;
    size_t capacity_;
};

int main() {
    static_assert(Vec<Vec<int>>::with(Vec<int>::with(1, 2, 3),  //
                                      Vec<int>::with(4),        //
                                      Vec<int>::with(),         //
                                      Vec<int>::with(5, 6))
                      .into_iter()
                      .flatten()
                      .sum() == 1 + 2 + 3 + 4 + 5 + 6);
}
```

[Bug c++/110905] GCC rejects constexpr code that may re-initialize union member

[ppalka at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110905

Patrick Palka <ppalka at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           See Also|                            |https://gcc.gnu.org/bugzill
                   |                            |a/show_bug.cgi?id=101631
                 CC|                            |ppalka at gcc dot gnu.org

--- Comment #5 from Patrick Palka <ppalka at gcc dot gnu.org> ---
GCC trunk accepts the comment #4 testcase (thanks for the reduction) since
r14-4771.