Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

clang-13 fails to compile libstdc++'s std::variant<std::vector<int>>: error: attempt to use a deleted function #52956

Closed
trofi opened this issue Jan 2, 2022 · 10 comments
Labels
c++20 clang:frontend Language frontend issues, e.g. anything involving "Sema" concepts C++20 concepts

Comments

@trofi
Copy link
Contributor

trofi commented Jan 2, 2022

Initially I observed the error as a mold-1.0.0 build failure on clang-13 which uses gcc-12's libstdc++. Here is a one line reproducer:

#include <variant>
#include <vector>

std::variant<std::vector<long> > v;

gcc-12 builds it fine, clang-13 fails:

$ /tmp/gcc-12/bin/c++ -std=c++20 -c tapi.cc
$ /tmp/clang-13/bin/c++ -std=c++20 -c tapi.cc
/<<NIX>>/gcc-12.0.0/include/c++/12.0.0/variant:460:2: error: attempt to use a deleted function
        _Variant_storage(in_place_index_t<_Np>, _Args&&... __args)
        ^
/<<NIX>>/gcc-12.0.0/include/c++/12.0.0/variant:747:4: note: in instantiation of function template specialization 'std::__detail::__variant::_Variant_storage<false, std::vector<long>>::_Variant_storage<0UL>' requested here
        : _Base(__i, std::forward<_Args>(__args)...)
          ^
/<<NIX>>/gcc-12.0.0/include/c++/12.0.0/variant:742:9: note: in instantiation of function template specialization 'std::__detail::__variant::_Variant_base<std::vector<long>>::_Variant_base<0UL>' requested here
      : _Variant_base(in_place_index<0>) { }
        ^
/<<NIX>>/gcc-12.0.0/include/c++/12.0.0/variant:1403:7: note: in instantiation of member function 'std::__detail::__variant::_Variant_base<std::vector<long>>::_Variant_base' requested here
      variant() = default;
      ^
tapi.cc:13:34: note: in defaulted default constructor for 'std::variant<std::vector<long>>' first required here
std::variant<std::vector<long> > v;
                                 ^
/<<NIX>>/gcc-12.0.0/include/c++/12.0.0/variant:401:7: note: explicitly defaulted function was implicitly deleted here
      ~_Variadic_union() = default;
      ^
/<<NIX>>/gcc-12.0.0/include/c++/12.0.0/variant:409:30: note: destructor of '_Variadic_union<std::vector<long>>' is implicitly deleted because variant field '_M_first' has a non-trivial destructor
      _Uninitialized<_First> _M_first;
                             ^
/<<NIX>>/gcc-12.0.0/include/c++/12.0.0/variant:480:7: error: attempt to use a deleted function
      ~_Variant_storage()
      ^
/<<NIX>>/gcc-12.0.0/include/c++/12.0.0/variant:558:20: note: in instantiation of member function 'std::__detail::__variant::_Variant_storage<false, std::vector<long>>::~_Variant_storage' requested here
      using _Base::_Base;
                   ^
/<<NIX>>/gcc-12.0.0/include/c++/12.0.0/variant:742:9: note: in instantiation of function template specialization 'std::__detail::__variant::_Variant_base<std::vector<long>>::_Variant_base<0UL>' requested here
      : _Variant_base(in_place_index<0>) { }
        ^
/<<NIX>>/gcc-12.0.0/include/c++/12.0.0/variant:1403:7: note: in instantiation of member function 'std::__detail::__variant::_Variant_base<std::vector<long>>::_Variant_base' requested here
      variant() = default;
      ^
tapi.cc:13:34: note: in defaulted default constructor for 'std::variant<std::vector<long>>' first required here
std::variant<std::vector<long> > v;
                                 ^
/<<NIX>>/gcc-12.0.0/include/c++/12.0.0/variant:401:7: note: explicitly defaulted function was implicitly deleted here
      ~_Variadic_union() = default;
      ^
/<<NIX>>/gcc-12.0.0/include/c++/12.0.0/variant:409:30: note: destructor of '_Variadic_union<std::vector<long>>' is implicitly deleted because variant field '_M_first' has a non-trivial destructor
      _Uninitialized<_First> _M_first;
                             ^
2 errors generated.

I attempted to shrink down preprocessed file to extract compiler difference. My result:

struct _Vector_base {
  ~_Vector_base();
};
template <typename> union _Variadic_union {
  ~_Variadic_union() requires(!__has_trivial_destructor(_Variadic_union));
  _Vector_base _M_first;
};
struct _Variant_storage {
  _Variant_storage();
  _Variadic_union<int> _M_u;
};
struct _Variant_storage v;

gcc / clang handling difference:

$ /tmp/gcc-12/bin/c++ -std=c++20 -c tapi.cc.cc
$ /tmp/clang-13/bin/c++ -std=c++20 -c tapi.cc.cc
tapi.cc.cc:6:32: error: incomplete type '_Variadic_union<int>' used in type trait expression
  ~_Variadic_union() requires(!__has_trivial_destructor(_Variadic_union));
                               ^
tapi.cc.cc:11:24: note: in instantiation of template class '_Variadic_union<int>' requested here
  _Variadic_union<int> _M_u;
                       ^
tapi.cc.cc:5:27: note: definition of '_Variadic_union<int>' is not complete until the closing '}'
template <typename> union _Variadic_union {
                          ^
tapi.cc.cc:13:25: error: attempt to use a deleted function
struct _Variant_storage v;
                        ^
tapi.cc.cc:11:24: note: destructor of '_Variant_storage' is implicitly deleted because field '_M_u' has a deleted destructor
  _Variadic_union<int> _M_u;
                       ^
tapi.cc.cc:7:16: note: destructor of '_Variadic_union<int>' is implicitly deleted because variant field '_M_first' has a non-trivial destructor
  _Vector_base _M_first;
               ^
2 errors generated.
@rengolin
Copy link
Member

rengolin commented Jan 2, 2022

The errors are not the same on your reduced version. You must check for error: attempt to use a deleted function and make sure it's still in the constructor _Variant_storage(in_place_index_t<_Np>, _Args&&... __args).

I suspect this isn't a compiler bug, per se, but how clang and gcc interpret incomplete or unimplemented types, as you can clearly see in your reduced version.

@trofi
Copy link
Contributor Author

trofi commented Jan 2, 2022

Aha, thank you! I'm not really sure not to get better/smaller example. Filed https://gcc.gnu.org/PR103891 to possibly get some help from libstdc++ developers (and maybe fix libstdc++).

@rengolin
Copy link
Member

rengolin commented Jan 3, 2022

I normally use creduce(https://embed.cs.utah.edu/creduce/) to reduce cases, with a script that greps for the particular error message.

On the GCC front, Andrew seems to have found a promising direction on what looks to be a similar issue. But I'm copying some people in case he's right about missing C++ 20 features.

@rnk @zygoloid @dwblaikie @davidchisnall

@aaronpuchert aaronpuchert added c++20 clang:frontend Language frontend issues, e.g. anything involving "Sema" and removed new issue labels Jan 3, 2022
@aaronpuchert
Copy link
Member

The error message is different, but the original ends with “destructor [...] is implicitly deleted because variant field _M_first has a non-trivial destructor” and it doesn't tell us why. Probably some length limit for the error message that we don't hit in the reduced code.

Maybe @saarraz can help. Question here seems to be whether constraints should be evaluated after completing the class definition (like member function bodies) or not. Not sure where the standard would be defining that.

@aaronpuchert
Copy link
Member

Seems that [class.mem.general]p7 says that a requires clause isn't a complete-class context:

A complete-class context of a class (template) is a

  • function body ([dcl.fct.def.general]),
  • default argument ([dcl.fct.default]),
  • default template argument ([temp.param]),
  • noexcept-specifier ([except.spec]), or
  • default member initializer

within the member-specification of the class or class template.

So I think Clang is right here. But maybe I'm missing something.

@aaronpuchert
Copy link
Member

In fact, isn't it pretty obvious that it can't be a complete-class context? Because that could affect the availability of member functions, which in turn affects some type traits. Consider the negation:

template <typename> union _Variadic_union {
  ~_Variadic_union() requires(__has_trivial_destructor(_Variadic_union));
};

So if __has_trivial_destructor is true, we declare our own destructor, thus contradicting the predicate our decision was based on. If it isn't, we don't declare a destructor, which leads again to a contradiction.

With noexcept specifiers Clang solves this by not allowing references to the class itself or to member functions that are declared later (so in some sense it's more of an opportunistic complete-class context):

template <typename> struct X {
  ~X() noexcept(noexcept(X()));
  static void f() noexcept(noexcept(g()));
  static void g() noexcept(noexcept(f()));
};

X<int> x;

produces:

<source>:3:37: error: exception specification is not available until end of class definition
  static void f() noexcept(noexcept(g()));
                                    ^
<source>:2:26: error: exception specification of '~X' uses itself
  ~X() noexcept(noexcept(X()));
                         ^
<source>:2:26: note: in instantiation of exception specification for '~X' requested here
<source>:7:8: note: in instantiation of exception specification for '~X' requested here
X<int> x;
       ^

@aaronpuchert
Copy link
Member

aaronpuchert commented Jan 4, 2022

The original code has no self-reference though:

  template<typename _First, typename... _Rest>
    union _Variadic_union<_First, _Rest...>
    {
      // ...
      constexpr ~_Variadic_union()
        requires (!__has_trivial_destructor(_First))
              || (!__has_trivial_destructor(_Variadic_union<_Rest...>))
      { }
      // ...
    };

So while I think Clang is right to reject the reduction (not sure how GCC is able to compile this), the original code should be fine. And as @jwakely pointed out in the GCC bug report, it's very likely #45614.

@jwakely
Copy link
Contributor

jwakely commented Jan 5, 2022

@rengolin

I normally use creduce(https://embed.cs.utah.edu/creduce/) to reduce cases, with a script that greps for the particular error message.

https://github.com/marxin/cvise is sometimes much faster than the original creduce. You can use exactly the same "interestingness test" scripts with it.

@trofi
Copy link
Contributor Author

trofi commented Jan 9, 2022

I'm not comfortable reducing original example because libstdc++ is full of version ifdefs. I don't think I can construct plausible test for it. I'm not sure where clang++/g++ is expected to be different and where it's accidental.

I'm attaching mostly selfcontained archive with minimal example and bundled libstdc++ for x86_64: src.tar.gz.

I was not able to reduce it even manually as I got lost in various #if __cpp_lib_variant >= 202106L. From what I understood from partial reduction <variant> does rely on destructor selection related to #45614.

$ 
./bug.bash
CLANG++
In file included from bug.cc:12:
stdc++-include/c++/12.0.0/variant:300:18: error: no template named '__aligned_membuf' in namespace '__gnu_cxx'
      __gnu_cxx::__aligned_membuf<_Type> _M_storage;
      ~~~~~~~~~~~^
stdc++-include/c++/12.0.0/variant:409:30: error: implicit instantiation of undefined template 'std::__detail::__variant::_Uninitialized<std::vector<long>, false>'
      _Uninitialized<_First> _M_first;
                             ^
stdc++-include/c++/12.0.0/variant:491:34: note: in instantiation of template class 'std::__detail::__variant::_Variadic_union<std::vector<long>>' requested here
      _Variadic_union<_Types...> _M_u;
                                 ^
stdc++-include/c++/12.0.0/variant:555:30: note: in instantiation of template class 'std::__detail::__variant::_Variant_storage<false, std::vector<long>>' requested here
    struct _Copy_ctor_base : _Variant_storage_alias<_Types...>
                             ^
stdc++-include/c++/12.0.0/variant:592:30: note: in instantiation of template class 'std::__detail::__variant::_Copy_ctor_base<false, std::vector<long>>' requested here
    struct _Move_ctor_base : _Copy_ctor_alias<_Types...>
                             ^
stdc++-include/c++/12.0.0/variant:630:32: note: in instantiation of template class 'std::__detail::__variant::_Move_ctor_base<false, std::vector<long>>' requested here
    struct _Copy_assign_base : _Move_ctor_alias<_Types...>
                               ^
stdc++-include/c++/12.0.0/variant:682:32: note: in instantiation of template class 'std::__detail::__variant::_Copy_assign_base<false, std::vector<long>>' requested here
    struct _Move_assign_base : _Copy_assign_alias<_Types...>
                               ^
stdc++-include/c++/12.0.0/variant:736:28: note: in instantiation of template class 'std::__detail::__variant::_Move_assign_base<false, std::vector<long>>' requested here
    struct _Variant_base : _Move_assign_alias<_Types...>
                           ^
stdc++-include/c++/12.0.0/variant:1338:15: note: in instantiation of template class 'std::__detail::__variant::_Variant_base<std::vector<long>>' requested here
    : private __detail::__variant::_Variant_base<_Types...>,
              ^
bug.cc:16:16: note: in instantiation of template class 'std::variant<std::vector<long>>' requested here
    return new std::variant<std::vector<long> >;
               ^
stdc++-include/c++/12.0.0/variant:218:12: note: template is declared here
    struct _Uninitialized;
           ^
2 errors generated.
G++

@Quuxplusone Quuxplusone added the concepts C++20 concepts label Jan 16, 2022
@royjacobson
Copy link
Contributor

#45614 is fixed now and the example from the initial report compiles with up to date clang/libstdc++. Closing this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++20 clang:frontend Language frontend issues, e.g. anything involving "Sema" concepts C++20 concepts
Projects
Status: Done
Development

No branches or pull requests

6 participants