Bug 106176 - Compiler diagnostic doesn't show where it's coming from in my code
Summary: Compiler diagnostic doesn't show where it's coming from in my code
Status: NEW
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: 12.0
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords: diagnostic
: 106584 (view as bug list)
Depends on:
Blocks:
 
Reported: 2022-07-03 22:36 UTC by Barry Revzin
Modified: 2022-08-22 11:28 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2022-08-22 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Barry Revzin 2022-07-03 22:36:30 UTC
Consider the following program, carefully reduced from real code:

#include <map>

// template <class T> using which = std::map<int, T>;
template <class T> using which = T;

struct C
{
    C();
    ~C() = default;

    static C create();   

    struct M
    {
        M(M&&) = default;
    };

    which<M> member;
};

C C::create()
{
    C c;
    return c;
}
 
This, when compiled with gcc 12.1 -std=c++20 emits (https://godbolt.org/z/YhfzWTjKq):

<source>: In static member function 'static C C::create()':
<source>:24:12: error: use of deleted function 'C::C(const C&)'
   24 |     return c;
      |            ^
<source>:6:8: note: 'C::C(const C&)' is implicitly deleted because the default definition would be ill-formed:
    6 | struct C
      |        ^
<source>:6:8: error: use of deleted function 'constexpr C::M::M(const C::M&)'
<source>:13:12: note: 'constexpr C::M::M(const C::M&)' is implicitly declared as deleted because 'C::M' declares a move constructor or move assignment operator
   13 |     struct M
      |            ^
Compiler returned: 1

This diagnostic (a) points to the offending line in my code, precisely, and (b) explains what the problem is. That's pretty good. It could be better: the real problem here is that C's explicitly default destructor inhibited the compiler-generated move constructor, why is why the copy is a problem to begin with. 

If we change the definition of the member from M to std::map<int, M> (by changing which declaration of which is commented), nothing really changes about this program - we still have a move-only member, so this is broken since C is missing the move constructor, etc. However, the new diagnostic becomes (https://godbolt.org/z/9TG3WK75G):

In file included from /opt/compiler-explorer/gcc-12.1.0/include/c++/12.1.0/ext/alloc_traits.h:34,
                 from /opt/compiler-explorer/gcc-12.1.0/include/c++/12.1.0/bits/stl_tree.h:67,
                 from /opt/compiler-explorer/gcc-12.1.0/include/c++/12.1.0/map:60,
                 from <source>:1:
/opt/compiler-explorer/gcc-12.1.0/include/c++/12.1.0/bits/alloc_traits.h: In instantiation of 'static constexpr void std::allocator_traits<std::allocator<_Up> >::construct(allocator_type&, _Up*, _Args&& ...) [with _Up = std::pair<const int, C::M>; _Args = {const std::pair<const int, C::M>&}; _Tp = std::_Rb_tree_node<std::pair<const int, C::M> >; allocator_type = std::allocator<std::_Rb_tree_node<std::pair<const int, C::M> > >]':
/opt/compiler-explorer/gcc-12.1.0/include/c++/12.1.0/bits/stl_tree.h:595:32:   required from 'void std::_Rb_tree<_Key, _Val, _KeyOfValue, _Compare, _Alloc>::_M_construct_node(_Link_type, _Args&& ...) [with _Args = {const std::pair<const int, C::M>&}; _Key = int; _Val = std::pair<const int, C::M>; _KeyOfValue = std::_Select1st<std::pair<const int, C::M> >; _Compare = std::less<int>; _Alloc = std::allocator<std::pair<const int, C::M> >; _Link_type = std::_Rb_tree_node<std::pair<const int, C::M> >*]'
/opt/compiler-explorer/gcc-12.1.0/include/c++/12.1.0/bits/stl_tree.h:612:21:   required from 'std::_Rb_tree_node<_Val>* std::_Rb_tree<_Key, _Val, _KeyOfValue, _Compare, _Alloc>::_M_create_node(_Args&& ...) [with _Args = {const std::pair<const int, C::M>&}; _Key = int; _Val = std::pair<const int, C::M>; _KeyOfValue = std::_Select1st<std::pair<const int, C::M> >; _Compare = std::less<int>; _Alloc = std::allocator<std::pair<const int, C::M> >; _Link_type = std::_Rb_tree_node<std::pair<const int, C::M> >*]'
/opt/compiler-explorer/gcc-12.1.0/include/c++/12.1.0/bits/stl_tree.h:529:32:   required from 'std::_Rb_tree_node<_Val>* std::_Rb_tree<_Key, _Val, _KeyOfValue, _Compare, _Alloc>::_Alloc_node::operator()(_Arg&&) const [with _Arg = const std::pair<const int, C::M>&; _Key = int; _Val = std::pair<const int, C::M>; _KeyOfValue = std::_Select1st<std::pair<const int, C::M> >; _Compare = std::less<int>; _Alloc = std::allocator<std::pair<const int, C::M> >; std::_Rb_tree<_Key, _Val, _KeyOfValue, _Compare, _Alloc>::_Link_type = std::_Rb_tree_node<std::pair<const int, C::M> >*]'
/opt/compiler-explorer/gcc-12.1.0/include/c++/12.1.0/bits/stl_tree.h:645:18:   required from 'std::_Rb_tree_node<_Val>* std::_Rb_tree<_Key, _Val, _KeyOfValue, _Compare, _Alloc>::_M_clone_node(_Link_type, _NodeGen&) [with bool _MoveValue = false; _NodeGen = std::_Rb_tree<int, std::pair<const int, C::M>, std::_Select1st<std::pair<const int, C::M> >, std::less<int>, std::allocator<std::pair<const int, C::M> > >::_Alloc_node; _Key = int; _Val = std::pair<const int, C::M>; _KeyOfValue = std::_Select1st<std::pair<const int, C::M> >; _Compare = std::less<int>; _Alloc = std::allocator<std::pair<const int, C::M> >; _Link_type = std::_Rb_tree_node<std::pair<const int, C::M> >*]'
/opt/compiler-explorer/gcc-12.1.0/include/c++/12.1.0/bits/stl_tree.h:1895:47:   required from 'std::_Rb_tree_node<_Val>* std::_Rb_tree<_Key, _Val, _KeyOfValue, _Compare, _Alloc>::_M_copy(_Link_type, _Base_ptr, _NodeGen&) [with bool _MoveValues = false; _NodeGen = std::_Rb_tree<int, std::pair<const int, C::M>, std::_Select1st<std::pair<const int, C::M> >, std::less<int>, std::allocator<std::pair<const int, C::M> > >::_Alloc_node; _Key = int; _Val = std::pair<const int, C::M>; _KeyOfValue = std::_Select1st<std::pair<const int, C::M> >; _Compare = std::less<int>; _Alloc = std::allocator<std::pair<const int, C::M> >; _Link_type = std::_Rb_tree_node<std::pair<const int, C::M> >*; _Base_ptr = std::_Rb_tree_node_base*]'
/opt/compiler-explorer/gcc-12.1.0/include/c++/12.1.0/bits/stl_tree.h:890:26:   required from 'std::_Rb_tree_node<_Val>* std::_Rb_tree<_Key, _Val, _KeyOfValue, _Compare, _Alloc>::_M_copy(const std::_Rb_tree<_Key, _Val, _KeyOfValue, _Compare, _Alloc>&, _NodeGen&) [with bool _MoveValues = false; _NodeGen = std::_Rb_tree<int, std::pair<const int, C::M>, std::_Select1st<std::pair<const int, C::M> >, std::less<int>, std::allocator<std::pair<const int, C::M> > >::_Alloc_node; _Key = int; _Val = std::pair<const int, C::M>; _KeyOfValue = std::_Select1st<std::pair<const int, C::M> >; _Compare = std::less<int>; _Alloc = std::allocator<std::pair<const int, C::M> >; _Link_type = std::_Rb_tree_node<std::pair<const int, C::M> >*]'
/opt/compiler-explorer/gcc-12.1.0/include/c++/12.1.0/bits/stl_tree.h:901:29:   required from 'std::_Rb_tree_node<_Val>* std::_Rb_tree<_Key, _Val, _KeyOfValue, _Compare, _Alloc>::_M_copy(const std::_Rb_tree<_Key, _Val, _KeyOfValue, _Compare, _Alloc>&) [with _Key = int; _Val = std::pair<const int, C::M>; _KeyOfValue = std::_Select1st<std::pair<const int, C::M> >; _Compare = std::less<int>; _Alloc = std::allocator<std::pair<const int, C::M> >; _Link_type = std::_Rb_tree_node<std::pair<const int, C::M> >*]'
/opt/compiler-explorer/gcc-12.1.0/include/c++/12.1.0/bits/stl_tree.h:939:23:   required from 'std::_Rb_tree<_Key, _Val, _KeyOfValue, _Compare, _Alloc>::_Rb_tree(const std::_Rb_tree<_Key, _Val, _KeyOfValue, _Compare, _Alloc>&) [with _Key = int; _Val = std::pair<const int, C::M>; _KeyOfValue = std::_Select1st<std::pair<const int, C::M> >; _Compare = std::less<int>; _Alloc = std::allocator<std::pair<const int, C::M> >]'
/opt/compiler-explorer/gcc-12.1.0/include/c++/12.1.0/bits/stl_map.h:217:7:   required from here
/opt/compiler-explorer/gcc-12.1.0/include/c++/12.1.0/bits/alloc_traits.h:518:28: error: no matching function for call to 'construct_at(std::pair<const int, C::M>*&, const std::pair<const int, C::M>&)'
  518 |           std::construct_at(__p, std::forward<_Args>(__args)...);
      |           ~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from /opt/compiler-explorer/gcc-12.1.0/include/c++/12.1.0/bits/stl_iterator.h:85,
                 from /opt/compiler-explorer/gcc-12.1.0/include/c++/12.1.0/bits/stl_algobase.h:67,
                 from /opt/compiler-explorer/gcc-12.1.0/include/c++/12.1.0/bits/stl_tree.h:63:
/opt/compiler-explorer/gcc-12.1.0/include/c++/12.1.0/bits/stl_construct.h:94:5: note: candidate: 'template<class _Tp, class ... _Args> constexpr decltype (::new(void*(0)) _Tp) std::construct_at(_Tp*, _Args&& ...)'
   94 |     construct_at(_Tp* __location, _Args&&... __args)
      |     ^~~~~~~~~~~~
/opt/compiler-explorer/gcc-12.1.0/include/c++/12.1.0/bits/stl_construct.h:94:5: note:   template argument deduction/substitution failed:
/opt/compiler-explorer/gcc-12.1.0/include/c++/12.1.0/bits/stl_construct.h: In substitution of 'template<class _Tp, class ... _Args> constexpr decltype (::new(void*(0)) _Tp) std::construct_at(_Tp*, _Args&& ...) [with _Tp = std::pair<const int, C::M>; _Args = {const std::pair<const int, C::M>&}]':
/opt/compiler-explorer/gcc-12.1.0/include/c++/12.1.0/bits/alloc_traits.h:518:21:   required from 'static constexpr void std::allocator_traits<std::allocator<_Up> >::construct(allocator_type&, _Up*, _Args&& ...) [with _Up = std::pair<const int, C::M>; _Args = {const std::pair<const int, C::M>&}; _Tp = std::_Rb_tree_node<std::pair<const int, C::M> >; allocator_type = std::allocator<std::_Rb_tree_node<std::pair<const int, C::M> > >]'
/opt/compiler-explorer/gcc-12.1.0/include/c++/12.1.0/bits/stl_tree.h:595:32:   required from 'void std::_Rb_tree<_Key, _Val, _KeyOfValue, _Compare, _Alloc>::_M_construct_node(_Link_type, _Args&& ...) [with _Args = {const std::pair<const int, C::M>&}; _Key = int; _Val = std::pair<const int, C::M>; _KeyOfValue = std::_Select1st<std::pair<const int, C::M> >; _Compare = std::less<int>; _Alloc = std::allocator<std::pair<const int, C::M> >; _Link_type = std::_Rb_tree_node<std::pair<const int, C::M> >*]'
/opt/compiler-explorer/gcc-12.1.0/include/c++/12.1.0/bits/stl_tree.h:612:21:   required from 'std::_Rb_tree_node<_Val>* std::_Rb_tree<_Key, _Val, _KeyOfValue, _Compare, _Alloc>::_M_create_node(_Args&& ...) [with _Args = {const std::pair<const int, C::M>&}; _Key = int; _Val = std::pair<const int, C::M>; _KeyOfValue = std::_Select1st<std::pair<const int, C::M> >; _Compare = std::less<int>; _Alloc = std::allocator<std::pair<const int, C::M> >; _Link_type = std::_Rb_tree_node<std::pair<const int, C::M> >*]'
/opt/compiler-explorer/gcc-12.1.0/include/c++/12.1.0/bits/stl_tree.h:529:32:   required from 'std::_Rb_tree_node<_Val>* std::_Rb_tree<_Key, _Val, _KeyOfValue, _Compare, _Alloc>::_Alloc_node::operator()(_Arg&&) const [with _Arg = const std::pair<const int, C::M>&; _Key = int; _Val = std::pair<const int, C::M>; _KeyOfValue = std::_Select1st<std::pair<const int, C::M> >; _Compare = std::less<int>; _Alloc = std::allocator<std::pair<const int, C::M> >; std::_Rb_tree<_Key, _Val, _KeyOfValue, _Compare, _Alloc>::_Link_type = std::_Rb_tree_node<std::pair<const int, C::M> >*]'
/opt/compiler-explorer/gcc-12.1.0/include/c++/12.1.0/bits/stl_tree.h:645:18:   required from 'std::_Rb_tree_node<_Val>* std::_Rb_tree<_Key, _Val, _KeyOfValue, _Compare, _Alloc>::_M_clone_node(_Link_type, _NodeGen&) [with bool _MoveValue = false; _NodeGen = std::_Rb_tree<int, std::pair<const int, C::M>, std::_Select1st<std::pair<const int, C::M> >, std::less<int>, std::allocator<std::pair<const int, C::M> > >::_Alloc_node; _Key = int; _Val = std::pair<const int, C::M>; _KeyOfValue = std::_Select1st<std::pair<const int, C::M> >; _Compare = std::less<int>; _Alloc = std::allocator<std::pair<const int, C::M> >; _Link_type = std::_Rb_tree_node<std::pair<const int, C::M> >*]'
/opt/compiler-explorer/gcc-12.1.0/include/c++/12.1.0/bits/stl_tree.h:1895:47:   required from 'std::_Rb_tree_node<_Val>* std::_Rb_tree<_Key, _Val, _KeyOfValue, _Compare, _Alloc>::_M_copy(_Link_type, _Base_ptr, _NodeGen&) [with bool _MoveValues = false; _NodeGen = std::_Rb_tree<int, std::pair<const int, C::M>, std::_Select1st<std::pair<const int, C::M> >, std::less<int>, std::allocator<std::pair<const int, C::M> > >::_Alloc_node; _Key = int; _Val = std::pair<const int, C::M>; _KeyOfValue = std::_Select1st<std::pair<const int, C::M> >; _Compare = std::less<int>; _Alloc = std::allocator<std::pair<const int, C::M> >; _Link_type = std::_Rb_tree_node<std::pair<const int, C::M> >*; _Base_ptr = std::_Rb_tree_node_base*]'
/opt/compiler-explorer/gcc-12.1.0/include/c++/12.1.0/bits/stl_tree.h:890:26:   required from 'std::_Rb_tree_node<_Val>* std::_Rb_tree<_Key, _Val, _KeyOfValue, _Compare, _Alloc>::_M_copy(const std::_Rb_tree<_Key, _Val, _KeyOfValue, _Compare, _Alloc>&, _NodeGen&) [with bool _MoveValues = false; _NodeGen = std::_Rb_tree<int, std::pair<const int, C::M>, std::_Select1st<std::pair<const int, C::M> >, std::less<int>, std::allocator<std::pair<const int, C::M> > >::_Alloc_node; _Key = int; _Val = std::pair<const int, C::M>; _KeyOfValue = std::_Select1st<std::pair<const int, C::M> >; _Compare = std::less<int>; _Alloc = std::allocator<std::pair<const int, C::M> >; _Link_type = std::_Rb_tree_node<std::pair<const int, C::M> >*]'
/opt/compiler-explorer/gcc-12.1.0/include/c++/12.1.0/bits/stl_tree.h:901:29:   required from 'std::_Rb_tree_node<_Val>* std::_Rb_tree<_Key, _Val, _KeyOfValue, _Compare, _Alloc>::_M_copy(const std::_Rb_tree<_Key, _Val, _KeyOfValue, _Compare, _Alloc>&) [with _Key = int; _Val = std::pair<const int, C::M>; _KeyOfValue = std::_Select1st<std::pair<const int, C::M> >; _Compare = std::less<int>; _Alloc = std::allocator<std::pair<const int, C::M> >; _Link_type = std::_Rb_tree_node<std::pair<const int, C::M> >*]'
/opt/compiler-explorer/gcc-12.1.0/include/c++/12.1.0/bits/stl_tree.h:939:23:   required from 'std::_Rb_tree<_Key, _Val, _KeyOfValue, _Compare, _Alloc>::_Rb_tree(const std::_Rb_tree<_Key, _Val, _KeyOfValue, _Compare, _Alloc>&) [with _Key = int; _Val = std::pair<const int, C::M>; _KeyOfValue = std::_Select1st<std::pair<const int, C::M> >; _Compare = std::less<int>; _Alloc = std::allocator<std::pair<const int, C::M> >]'
/opt/compiler-explorer/gcc-12.1.0/include/c++/12.1.0/bits/stl_map.h:217:7:   required from here
/opt/compiler-explorer/gcc-12.1.0/include/c++/12.1.0/bits/stl_construct.h:96:17: error: use of deleted function 'constexpr std::pair<_T1, _T2>::pair(const std::pair<_T1, _T2>&) [with _T1 = const int; _T2 = C::M]'
   96 |     -> decltype(::new((void*)0) _Tp(std::declval<_Args>()...))
      |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from /opt/compiler-explorer/gcc-12.1.0/include/c++/12.1.0/bits/stl_algobase.h:64:
/opt/compiler-explorer/gcc-12.1.0/include/c++/12.1.0/bits/stl_pair.h:195:17: note: 'constexpr std::pair<_T1, _T2>::pair(const std::pair<_T1, _T2>&) [with _T1 = const int; _T2 = C::M]' is implicitly deleted because the default definition would be ill-formed:
  195 |       constexpr pair(const pair&) = default;    ///< Copy constructor
      |                 ^~~~
/opt/compiler-explorer/gcc-12.1.0/include/c++/12.1.0/bits/stl_pair.h:195:17: error: use of deleted function 'constexpr C::M::M(const C::M&)'
<source>:13:12: note: 'constexpr C::M::M(const C::M&)' is implicitly declared as deleted because 'C::M' declares a move constructor or move assignment operator
   13 |     struct M
      |            ^
Compiler returned: 1

This diagnostic is, importantly, missing any reference to _where_ in my code C is being copied, which is why this took me so long to track down (the create function sure doesn't look like it's copying!) Instead, it provides me all these details about map's copy constructor but doesn't actually say that it's map's copy constructor: I only get a reference to pair's copy constructor (though at least the call stack has enough info to figure out that it's map's copy constructor that's the problem).

In short:

1. It would be hugely beneficial if the diagnostic pointing to the originating line in my code for the problem.
2. It would be nice to have if (a) the diagnostic could explain why I didn't have a move constructor (though possibly this is over-fitting to this particular bug)  or (b) the diagnostic could not dive into the depths of libstdc++ and just tell me that std::map<int, M> wasn't copyable because M wasn't copyable and stop at that point (this one doesn't feel like over-fitting though).
Comment 1 Jonathan Wakely 2022-07-04 12:47:13 UTC
(In reply to Barry Revzin from comment #0)
> 1. It would be hugely beneficial if the diagnostic pointing to the
> originating line in my code for the problem.

I think this is PR 80858 (and also PR 106029).

> 2. It would be nice to have if (a) the diagnostic could explain why I didn't
> have a move constructor (though possibly this is over-fitting to this
> particular bug)

Maybe PR 53234 (and others).

>  or (b) the diagnostic could not dive into the depths of
> libstdc++ and just tell me that std::map<int, M> wasn't copyable because M
> wasn't copyable and stop at that point (this one doesn't feel like
> over-fitting though).

With a custom allocator it might actually be valid though, because "copying" M is done by calling the allocator's construct function (or allocator_traits::construct which calls std::construct_at) which might do something funky (e.g. default construct a new object, ignoring the thing being copied). In the general case, we don't know what "copying" the map does, so we have to try it and see where it fails.

In reality, that never happens, and we could probably do something better for std::map using std::allocator. But where to do that check and what to check for isn't obvious. We could put it in the default allocator_traits::construct but that's only one stack frame above std::construct_at so isn't really going to reduce the diagnostic output much.

With this change:

--- /home/jwakely/gcc/13/include/c++/13.0.0/bits/stl_tree.h~    2022-07-04 13:39:45.482834753 +0100
+++ /home/jwakely/gcc/13/include/c++/13.0.0/bits/stl_tree.h     2022-07-04 13:39:42.575833073 +0100
@@ -935,10 +935,6 @@
       _Rb_tree(const _Rb_tree& __x)
       : _M_impl(__x._M_impl)
       {
+#if __cpp_if_constexpr
+        if constexpr (is_same_v<allocator_type, allocator<value_type>>)
+          static_assert( is_copy_constructible_v<value_type> );
+#endif
        if (__x._M_root() != 0)
          _M_root() = _M_copy(__x);
       }

The error becomes:

In file included from /home/jwakely/gcc/13/include/c++/13.0.0/map:60,
                 from 106176.C:1:
/home/jwakely/gcc/13/include/c++/13.0.0/bits/stl_tree.h: In instantiation of 'std::_Rb_tree<_Key, _Val, _KeyOfValue, _Compare, _Alloc>::_Rb_tree(const std::_Rb_tree<_Key, _Val, _KeyOfValue, _Compare, _Alloc>&) [with _Key = int; _Val = std::pair<const int, C::M>; _KeyOfValue = std::_Select1st<std::pair<const int, C::M> >; _Compare = std::less<int>; _Alloc = std::allocator<std::pair<const int, C::M> >]':
/home/jwakely/gcc/13/include/c++/13.0.0/bits/stl_map.h:217:7:   required from here
/home/jwakely/gcc/13/include/c++/13.0.0/bits/stl_tree.h:940:26: error: static assertion failed
  940 |           static_assert( is_copy_constructible_v<value_type> );
      |                          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/jwakely/gcc/13/include/c++/13.0.0/bits/stl_tree.h:940:26: note: 'std::is_copy_constructible_v<std::pair<const int, C::M> >' evaluates to false

This still doesn't tell you where the copy is required from in your own code (PR 80858) and is arguably worse in some ways, because using a static_assert before trying the std::construct_at call means you no longer get this part of the output:

/home/jwakely/gcc/13/include/c++/13.0.0/bits/stl_pair.h:197:17: error: use of deleted function 'C::M::M(const C::M&)'
106176.C:13:12: note: 'constexpr C::M::M(const C::M&)' is implicitly declared as deleted because 'C::M' declares a move constructor or move assignment operator
   13 |     struct M
      |            ^

Because the code no longer actually tries to use that copy constructor (except inside a SFINAE context in the implementation of std::is_constructible) that is not shown.
Comment 2 Jonathan Wakely 2022-08-22 11:25:57 UTC
*** Bug 106584 has been marked as a duplicate of this bug. ***
Comment 3 Jonathan Wakely 2022-08-22 11:26:55 UTC
I think this is fundamentally the same issue as PR 80858.