[Bug c++/80858] When trying to copy std::unordered_map illegally, error message doesn't tell what's wrong

[redi at gcc dot gnu.org via Gcc-bugs]

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

--- Comment #9 from Jonathan Wakely  ---
The problem isn't limited to implicitly-declared special members. If we adjust
comment 4 so that B::operator=(const B&) is user-declared but defined as
defaulted:

 struct Empty {
};

template
struct A
{
A =(const A&) 
{
T t(3);
return *this;
}   
};

class B
{   
A a;

public:
B& operator=(const B&) = default;
};

int main(void)
{
B b1, b2;
b1 = b2;
}


Then we get almost the same error. The only difference is that the "required
from here" line points to the defaulted operator= instead of to the class head,
i.e. we get
head.cc:19:12:   required from here
instead of:
head.cc:14:7:   required from here

Which is a little better, but it's still easy to miss that line among the other
diagnostics.

This would still be greatly improved by "required from the implicitly-defined
copy assignment operator" instead of "required from here".

tl;dr we should name the special member that we're generating a defaulted
definition for, instead of just saying "here".

That's what EDG does, see comment 5:

  detected during:
instantiation of "A ::operator=(const A &) [with
  T=Empty]" at line 22
implicit generation of "B ::operator=(const B &)" at line 22

[Bug c++/80858] When trying to copy std::unordered_map illegally, error message doesn't tell what's wrong

[redi at gcc dot gnu.org via Gcc-bugs]

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

--- Comment #8 from Jonathan Wakely  ---
(In reply to Jonathan Wakely from comment #7)
> :5:8:   required from here
> /opt/compiler-explorer/gcc-13.2.0/include/c++/13.2.0/bits/unique_ptr.h:97:23:
> error: invalid application of 'sizeof' to incomplete type 'A'
>97 | static_assert(sizeof(_Tp)>0,
>   |   ^~~

Separately, I have a patch to improve the static assertion here so that it
actually prints the static_assert's string literal, instead of giving an error
in the boolean condition. But that's specific to the unique_ptr case, and
doesn't help the comment 0 and comment 4 examples.

[Bug c++/80858] When trying to copy std::unordered_map illegally, error message doesn't tell what's wrong

[redi at gcc dot gnu.org via Gcc-bugs]

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

Jonathan Wakely  changed:

   What|Removed |Added

   See Also||https://gcc.gnu.org/bugzill
   ||a/show_bug.cgi?id=109551
   Last reconfirmed|2017-05-22 00:00:00 |2023-10-20

--- Comment #7 from Jonathan Wakely  ---
For the original unordered_map case, deep down inside all that diagnostic
output we have:

/usr/include/c++/7/bits/unordered_map.h:101:11:   required from here

This is pointing to this line:

class unordered_map


What GCC is trying to say is that the error comes from an implicitly-declared
copy assignment operator, but because there is no location to point to (because
it's not user-declared), it just points to the class head. Which is not
helpful.

For comment 4 we get:

c4.cc:14:7:   required from here


And as Steinar observed, that's the "class B" line.

On IRC nightstrike gave this example:

#include 

struct A;

struct B {
std::unique_ptr a;
B();
//~B(); // Uncomment to fix
};

struct C {
B b;
C(): b() {}
};

This gives a static assert inside default_delete, which is nedeed by
~unique_ptr, which is needed by "struct B":

In file included from
/opt/compiler-explorer/gcc-13.2.0/include/c++/13.2.0/memory:78,
 from :1:
/opt/compiler-explorer/gcc-13.2.0/include/c++/13.2.0/bits/unique_ptr.h: In
instantiation of 'void std::default_delete<_Tp>::operator()(_Tp*) const [with
_Tp = A]':
/opt/compiler-explorer/gcc-13.2.0/include/c++/13.2.0/bits/unique_ptr.h:404:17: 
 required from 'std::unique_ptr<_Tp, _Dp>::~unique_ptr() [with _Tp = A; _Dp =
std::default_delete]'
:5:8:   required from here
/opt/compiler-explorer/gcc-13.2.0/include/c++/13.2.0/bits/unique_ptr.h:97:23:
error: invalid application of 'sizeof' to incomplete type 'A'
   97 | static_assert(sizeof(_Tp)>0,
  |   ^~~

There's no clue that it's needed by the implicit B::~B(). Even if you do know
that GCC is trying to refer to the location of an implicitly-declared special
member (which has no location), you have to guess *which* special member.

I think these cases would be greatly improved if the "required from here"
instead said "required from the implicitly-defined copy assignment operator" or
"required from the implicitly-defined destructor". We could still point to the
unhelpful location of the class head, as that does no real harm. But saying
that the error is triggered by the implicit definition of a particular special
member would make these errors easier to understand.

Somehow highlighting that line so that it's not buried would be nice too.
Because there's no caret diagnostic associated with the "required from here"
line, it's easy to miss among the more verbose lines on either side of it.

[Bug c++/80858] When trying to copy std::unordered_map illegally, error message doesn't tell what's wrong

[redi at gcc dot gnu.org via Gcc-bugs]

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

Jonathan Wakely  changed:

   What|Removed |Added

 CC||accelerator0099 at gmail dot 
com

--- Comment #6 from Jonathan Wakely  ---
*** Bug 106584 has been marked as a duplicate of this bug. ***

[Bug c++/80858] When trying to copy std::unordered_map illegally, error message doesn't tell what's wrong

[redi at gcc dot gnu.org]

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

--- Comment #5 from Jonathan Wakely  ---
Yes, that's great, thanks.

As you observed, Clang doesn't indicate the location that triggers the invalid
instantiation, but EDG does, correctly pointing to line 22:

"um2.cc", line 9: error: no suitable constructor exists to convert from "int"
  to "Empty"
  T t(3);
  ^
  detected during:
instantiation of "A ::operator=(const A &) [with
  T=Empty]" at line 22
implicit generation of "B ::operator=(const B &)" at line 22

1 error detected in the compilation of "um2.cc".

[Bug c++/80858] When trying to copy std::unordered_map illegally, error message doesn't tell what's wrong

[sgunderson at bigfoot dot com]

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

--- Comment #4 from sgunderson at bigfoot dot com ---
I think this should work as reduction:

struct Empty {
};

template
struct A
{
A =(const A&) 
{
T t(3);
return *this;
}   
};

class B
{   
A a;
};

int main(void)
{
B b1, b2;
b1 = b2;
}


The error is attributed to the line with “class B”, without ever mentioning the
“b1 = b2;” line.

[Bug c++/80858] When trying to copy std::unordered_map illegally, error message doesn't tell what's wrong

[redi at gcc dot gnu.org]

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

Jonathan Wakely  changed:

   What|Removed |Added

   Keywords||diagnostic
 Status|UNCONFIRMED |NEW
   Last reconfirmed||2017-05-22
 Ever confirmed|0   |1

--- Comment #3 from Jonathan Wakely  ---
Confirmed. I'll try to reduce the example.

[Bug c++/80858] When trying to copy std::unordered_map illegally, error message doesn't tell what's wrong

[sgunderson at bigfoot dot com]

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

--- Comment #2 from sgunderson at bigfoot dot com ---
Yes, I mean that the error message isn't clear (and it's basically the same
error message in 4.8, so no regression).

I don't think I understand the difficulties involved. Doesn't the error come as
a direct result of my copying? If I do this with e.g. std::vector, I get a much
clearer error message, which directly points to the line in question:

[…]
/usr/include/c++/6/bits/vector.tcc:195:19:   required from ‘std::vector<_Tp,
_Alloc>& std::vector<_Tp, 
_Alloc>::operator=(const std::vector<_Tp, _Alloc>&) [with _Tp =
std::unique_ptr; _Alloc = std::all
ocator]’
test.cc:7:7:   required from here

My main gripe is that with unordered_map, the error traceback stops in the
internal details of _Hashtable:

/usr/include/c++/7/bits/unordered_map.h:101:11:   required from here

In the real-world case in question, I eventually had to go into unordered_map.h
 (yes, in /usr/include) and replace “= default;” with “= delete;” to figure out
who was calling the copy constructor.

[Bug c++/80858] When trying to copy std::unordered_map illegally, error message doesn't tell what's wrong

[redi at gcc dot gnu.org]

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

--- Comment #1 from Jonathan Wakely  ---
(In reply to sgunderson from comment #0)
> Using gcc version 7.1.0 (Debian 7.1.0-5) (but the error goes back to at
> least 4.8, and amazingly, also in Clang), on this piece of code, simplified
> from a much bigger test case:

What error goes back to 4.8?

As you say, it's correct that the code doesn't compile. Do you mean the fact
that the error message isn't very clear, or something else?

In practice it's quite difficult to check the copyability (which would be
needed to produce a more helpful diagnostic) because it depends on the
allocator as well as the value_type.