[Bug c++/105683] New: [12 Regression] Infinite loop with construction of vector of variant

[Bug c++/105683] New: [12 Regression] Infinite loop with construction of vector of variant

[jeanmichael.celerier at gmail dot com]

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

            Bug ID: 105683
           Summary: [12 Regression] Infinite loop with construction of
                    vector of variant
           Product: gcc
           Version: 12.1.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: jeanmichael.celerier at gmail dot com
  Target Milestone: ---

Created attachment 53012
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=53012&action=edit
Full repro

Hello,

Full repro attached (g++ -std=gnu++20 repro.cpp && ./a.out is enough to trigger
the issue).
Same code works fine for every GCC version < 12 in c++17, c++20, as well as
with every released clang version in c++17, c++20, with both libc++ and
libstdc++.

My code more-or-less looks like this (trying to keep only the relevant parts):



    // line 58584
    struct value_variant_type {
      union Impl { std::vector<ossia::value> m_value8; } m_impl;

      value_variant_type(const std::vector<ossia::value>& v); 
      value_variant_type(std::vector<ossia::value>&& v);
    };


    // line 58681
    class value {
      value_variant_type v;
      value(const std::vector<ossia::value>& val) noexcept : v{val}
      {
      }
      explicit value(std::vector<ossia::value>&& val) noexcept :
v{std::move(val)}
      {
      }
    };



    // line 75925
    inline value_variant_type::value_variant_type(const
std::vector<ossia::value>& v) : m_type{Type8}
    {
      new (&m_impl.m_value8) std::vector<ossia::value>(v);
    }

    inline value_variant_type::value_variant_type(std::vector<ossia::value>&&
v) : m_type{Type8}
    {
      new (&m_impl.m_value8) std::vector<ossia::value>(std::move(v));
    }




    // line 58916
    inline ossia::value init_value(ossia::val_type type)
    {
      return std::vector<ossia::value>{};
    }



    // line 76232
    void create_value_inline(ossia::value& v, ossia::val_type t) {
      v = ossia::init_value(t);
    }

    int main()
    {
        ossia::value v;
        create_value_inline(v, ossia::val_type::LIST);
    }


What it looks that is happening is a loop between the construction of the value
and of the vector: 


    #1822 0x00005555555587b7 in ossia::value::value (other=...,
this=0x555555abff10)
    at
/home/jcelerier/ossia/score/3rdparty/libossia/src/ossia/network/value/value.hpp:302
    #1823 std::_Construct<ossia::value, ossia::value const&>
(__p=0x555555abff10) at /usr/include/c++/12.1.0/bits/stl_construct.h:119
    #1824 std::__do_uninit_copy<ossia::value const*, ossia::value*>
(__result=<optimized out>, __last=<optimized out>, __first=0x7fffff802a50)
    at /usr/include/c++/12.1.0/bits/stl_uninitialized.h:120
    #1825 std::__uninitialized_copy<false>::__uninit_copy<ossia::value const*,
ossia::value*> (__result=<optimized out>, __last=<optimized out>, 
    __first=<optimized out>) at
/usr/include/c++/12.1.0/bits/stl_uninitialized.h:137
    #1826 std::uninitialized_copy<ossia::value const*, ossia::value*>
(__result=<optimized out>, __last=<optimized out>, __first=<optimized out>)
    at /usr/include/c++/12.1.0/bits/stl_uninitialized.h:185
    #1827 std::__uninitialized_copy_a<ossia::value const*, ossia::value*,
ossia::value> (__result=0x555555abff10, __last=0x7fffff802a78,
__first=0x7fffff802a50)
    at /usr/include/c++/12.1.0/bits/stl_uninitialized.h:372
    #1828 std::vector<ossia::value, std::allocator<ossia::value>
>::_M_range_initialize<ossia::value const*> (__last=0x7fffff802a78,
__first=0x7fffff802a50, 
    this=0x555555abfee0) at /usr/include/c++/12.1.0/bits/stl_vector.h:1690
    #1829 std::vector<ossia::value, std::allocator<ossia::value> >::vector
(__a=..., Python Exception <class 'gdb.error'>: value has been optimized out
    __l=, this=0x555555abfee0) at /usr/include/c++/12.1.0/bits/stl_vector.h:677
    #1830 ossia::value_variant_type::value_variant_type (other=...,
this=0x555555abfee0)
    at
/home/jcelerier/ossia/score/3rdparty/libossia/src/ossia/network/value/value_variant_impl.hpp:17017
    #1831 ossia::value::value (other=..., this=0x555555abfee0) at
/home/jcelerier/ossia/score/3rdparty/libossia/src/ossia/network/value/value.hpp:302

[Bug c++/105683] [12/13 Regression] Infinite loop (at runtime) with construction of vector of variant

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[12 Regression] Infinite    |[12/13 Regression] Infinite
                   |loop with construction of   |loop (at runtime) with
                   |vector of variant           |construction of vector of
                   |                            |variant
           Keywords|                            |wrong-code
   Target Milestone|---                         |12.2

[Bug c++/105683] [12/13 Regression] Infinite loop (at runtime) with construction of vector of variant

[pinskia at gcc dot gnu.org]

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

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
It is calling the copy constructor (which you missed in your comment):
  value(const value& other) noexcept : v{other.v}
  {
  }


Which is called via:

    inline value_variant_type::value_variant_type(const
std::vector<ossia::value>& v) : m_type{Type8}
    {
      new (&m_impl.m_value8) std::vector<ossia::value>(v);
    }

inline ossia::value init_value(ossia::val_type type)
{
...
      return std::vector<ossia::value>{};
}


I am thinking there is a constructor that is not being elided here.

[Bug c++/105683] [12/13 Regression] Infinite loop (at runtime) with construction of vector of variant

[redi at gcc dot gnu.org]

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

--- Comment #2 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Don't use braced-init here:

      case Type::Type8:
        new (&m_impl.m_value8)
            std::vector<ossia::value>{std::move(other.m_impl.m_value8)};


If you use parens, it works as you expect.

[Bug c++/105683] [12/13 Regression] Infinite loop (at runtime) with construction of vector of variant

[redi at gcc dot gnu.org]

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

--- Comment #3 from Jonathan Wakely <redi at gcc dot gnu.org> ---
So this looks like another variation of PR 83264.

[Bug c++/105683] Infinite loop (at runtime) with construction of vector of variant

[redi at gcc dot gnu.org]

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

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[12/13 Regression] Infinite |Infinite loop (at runtime)
                   |loop (at runtime) with      |with construction of vector
                   |construction of vector of   |of variant
                   |variant                     |

--- Comment #4 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Why is this marked as a regression? It fails for gcc-9, 10, and 11 too. I don't
think this is a regression at all, I think it's just a dup of PR 83264.

[Bug c++/105683] Infinite loop (at runtime) with construction of vector of variant

[redi at gcc dot gnu.org]

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

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |DUPLICATE
             Status|UNCONFIRMED                 |RESOLVED

--- Comment #5 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Reduced:

#include <vector>

namespace ossia
{
    struct value;

    struct value_variant_type {
        union Impl {
            std::vector<ossia::value> m_value8;
            Impl() { }
            ~Impl() { }
        } m_impl;

        value_variant_type() { }
        value_variant_type(const value_variant_type&) { }

        value_variant_type(const std::vector<ossia::value>& v); 
        value_variant_type(std::vector<ossia::value>&& v);

        value_variant_type& operator=(const value_variant_type&)
        { return *this; }

        value_variant_type& operator=(value_variant_type&& other);
    };

    struct value {
        value_variant_type v;

        value() { }

        value(const std::vector<ossia::value>& val) noexcept : v{val}
        {
        }

        explicit value(std::vector<ossia::value>&& val) noexcept :
v{std::move(val)}
        {
        }
    };


    inline value_variant_type::value_variant_type(const
std::vector<ossia::value>& v)
    {
        new (&m_impl.m_value8) std::vector<ossia::value>{v};
    }

    inline value_variant_type::value_variant_type(std::vector<ossia::value>&&
v)
    {
        new (&m_impl.m_value8) std::vector<ossia::value>{std::move(v)};
    }


    value_variant_type& value_variant_type::operator=(value_variant_type&&
other)
    {
        new (&m_impl.m_value8)
            std::vector<ossia::value>{std::move(other.m_impl.m_value8)};
        return *this;
    }

    inline ossia::value init_value()
    {
        return std::vector<ossia::value>{};
    }

    void create_value_inline(ossia::value& v) {
        v = ossia::init_value();
    }
}

int main()
{
    ossia::value v;
    create_value_inline(v);
}


The problem is that you have vector<value>{val} where val can be converted to
type value. The standard says that this should be interpreted as an
initializer-list for the value elements of the vector (even though val is
actually vector<value> and so you might copy construction). See PR 83264 for
the relevant Core defect reports.

My suggestion is to only use braced-init if you *really* want it, i.e. you are
specifically trying to call an initializer-list constructor, or you *really*
want narrowing checks.

For a type like vector with an initializer-list constructor, and a value type
that is constructible from everything under the sun (or like here, a specific
set of types that includes the vector<value> type), using braced-init is more
trouble than it's worth. It has no advantage, and serious disadvantages.

Anybody who tells you to always used braced-init because it's superior is
wrong.

*** This bug has been marked as a duplicate of bug 83264 ***

[Bug c++/105683] Infinite loop (at runtime) with construction of vector of variant

[redi at gcc dot gnu.org]

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

--- Comment #6 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Jonathan Wakely from comment #5)
> The problem is that you have vector<value>{val} where val can be converted
> to type value. The standard says that this should be interpreted as an
> initializer-list for the value elements of the vector (even though val is
> actually vector<value> and so you might copy construction).

Oops, I meant to say "so you might _expect_ copy construction".