[Bug c++/88512] Too much STL in error output

2019-02-15 Thread redi at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88512

--- Comment #10 from Jonathan Wakely  ---
I've created bug 89370 to request showing the type as std::string.

[Bug c++/88512] Too much STL in error output

2019-02-15 Thread redi at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88512

--- Comment #9 from Jonathan Wakely  ---
It's easy for you to say that after looking at the reason it failed and knowing
what the code is trying to do (obviously "insert" modifies the string, so it
can't be const). But the compiler has to try every overload, and doesn't know
what your intention is, or what the word "insert" means.

The problem here is (as before) that std::string has eight overloads of insert
(and just as many for assign, and begin, and replace etc), and there's nothing
the compiler can do about that.

Potentially the compiler could be changed so that if all the overload
candidates are non-const and the object is const, it just says that. But that's
another fairly specialized diagnostic just for this case. Most classes simply
don't have eight overloads of the same function.

[Bug c++/88512] Too much STL in error output

2019-02-15 Thread jg at jguk dot org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88512

--- Comment #12 from Jonny Grant  ---
(In reply to Jonathan Wakely from comment #9)
> It's easy for you to say that after looking at the reason it failed and
> knowing what the code is trying to do (obviously "insert" modifies the
> string, so it can't be const). But the compiler has to try every overload,
> and doesn't know what your intention is, or what the word "insert" means.
> 
> The problem here is (as before) that std::string has eight overloads of
> insert (and just as many for assign, and begin, and replace etc), and
> there's nothing the compiler can do about that.
> 
> Potentially the compiler could be changed so that if all the overload
> candidates are non-const and the object is const, it just says that. But
> that's another fairly specialized diagnostic just for this case. Most
> classes simply don't have eight overloads of the same function.

Sounds good, those insert methods aren't 'const' so at least the first line
could say that...

eg:
$ g++ -Wall -c -o int int.cpp
int.cpp: In function ‘int main()’:
int.cpp:13:21: error: no matching function for call to
‘std::string::insert(size_t, const string&) const’
   str.insert(4, str2);
 ^
   Note, ‘str’ is const and all ‘std::string::insert()’ methods are non-const

[Bug c++/88512] Too much STL in error output

2019-02-15 Thread redi at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88512

--- Comment #11 from Jonathan Wakely  ---
There's also Bug 53281

[Bug c++/88512] Too much STL in error output

2019-02-15 Thread jg at jguk dot org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88512

--- Comment #8 from Jonny Grant  ---
Another example. 9 line file.  Gives 48 lines of STL warnings...  G++ only
needs to say something clear:

"error: std::string 'str' is const, and as such insert() method cannot be
called"

// g++ -Wall -c -o int int.cpp
#include 
int main()
{
  const std::string str ="abc";// NB. This 'str' should not be const
  const std::string str2 ="1 ";
  str.insert(4, str2);
  return str.size();
}


G++8 output:

$ g++-8 -Wall -c -o int int.cpp
int.cpp: In function ‘int main()’:
int.cpp:13:21: error: no matching function for call to
‘std::__cxx11::basic_string::insert(int, const string&) const’
   str.insert(4, str2);
 ^
In file included from /usr/include/c++/8/string:52,
 from int.cpp:3:
/usr/include/c++/8/bits/basic_string.h:1524:7: note: candidate:
‘std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::iterator
std::__cxx11::basic_string<_CharT, _Traits,
_Alloc>::insert(std::__cxx11::basic_string<_CharT, _Traits,
_Alloc>::const_iterator, std::__cxx11::basic_string<_CharT, _Traits,
_Alloc>::size_type, _CharT) [with _CharT = char; _Traits =
std::char_traits; _Alloc = std::allocator;
std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::iterator =
__gnu_cxx::__normal_iterator >;
typename __gnu_cxx::__alloc_traits::rebind<_CharT>::other>::pointer = char*;
std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::const_iterator =
__gnu_cxx::__normal_iterator >;
typename __gnu_cxx::__alloc_traits::rebind<_CharT>::other>::const_pointer =
const char*; std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::size_type =
long unsigned int]’
   insert(const_iterator __p, size_type __n, _CharT __c)
   ^~
/usr/include/c++/8/bits/basic_string.h:1524:7: note:   candidate expects 3
arguments, 2 provided
/usr/include/c++/8/bits/basic_string.h:1568:9: note: candidate: ‘template std::__cxx11::basic_string<_CharT, _Traits,
_Alloc>::iterator std::__cxx11::basic_string<_CharT, _Traits,
_Alloc>::insert(std::__cxx11::basic_string<_CharT, _Traits,
_Alloc>::const_iterator, _InputIterator, _InputIterator) [with _InputIterator =
_InputIterator;  = ; _CharT =
char; _Traits = std::char_traits; _Alloc = std::allocator]’
 insert(const_iterator __p, _InputIterator __beg, _InputIterator __end)
 ^~
/usr/include/c++/8/bits/basic_string.h:1568:9: note:   template argument
deduction/substitution failed:
int.cpp:13:21: note:   candidate expects 3 arguments, 2 provided
   str.insert(4, str2);
 ^
In file included from /usr/include/c++/8/string:52,
 from int.cpp:3:
/usr/include/c++/8/bits/basic_string.h:1602:7: note: candidate: ‘void
std::__cxx11::basic_string<_CharT, _Traits,
_Alloc>::insert(std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::iterator,
std::initializer_list<_Tp>) [with _CharT = char; _Traits =
std::char_traits; _Alloc = std::allocator;
std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::iterator =
__gnu_cxx::__normal_iterator >;
typename __gnu_cxx::__alloc_traits::rebind<_CharT>::other>::pointer = char*]’
   insert(iterator __p, initializer_list<_CharT> __l)
   ^~
/usr/include/c++/8/bits/basic_string.h:1602:7: note:   no known conversion for
argument 1 from ‘int’ to ‘std::__cxx11::basic_string::iterator’ {aka
‘__gnu_cxx::__normal_iterator >’}
/usr/include/c++/8/bits/basic_string.h:1622:7: note: candidate:
‘std::__cxx11::basic_string<_CharT, _Traits, _Alloc>&
std::__cxx11::basic_string<_CharT, _Traits,
_Alloc>::insert(std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::size_type,
const std::__cxx11::basic_string<_CharT, _Traits, _Alloc>&) [with _CharT =
char; _Traits = std::char_traits; _Alloc = std::allocator;
std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::size_type = long unsigned
int]’ 
   insert(size_type __pos1, const basic_string& __str)
   ^~
/usr/include/c++/8/bits/basic_string.h:1622:7: note:   passing ‘const string*’
{aka ‘const std::__cxx11::basic_string*’} as ‘this’ argument discards
qualifiers
/usr/include/c++/8/bits/basic_string.h:1645:7: note: candidate:
‘std::__cxx11::basic_string<_CharT, _Traits, _Alloc>&
std::__cxx11::basic_string<_CharT, _Traits,
_Alloc>::insert(std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::size_type,
const std::__cxx11::basic_string<_CharT, _Traits, _Alloc>&,
std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::size_type,
std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::size_type) [with _CharT =
char; _Traits = std::char_traits; _Alloc = std::allocator;
std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::size_type = long unsigned
int]’
   insert(size_type __pos1, const basic_string& __str,
   ^~
/usr/include/c++/8/bits/basic_string.h:1645:7: note:   candidate expects 4
arguments, 2 provided
/usr/include/c++/8/bits/basic_string.h:1668:7: note: candidate:
‘std::__cxx11::basic_string<_CharT, _Traits, _Alloc>&
std::__cxx11::basic_string<_CharT, _Traits,

[Bug c++/88512] Too much STL in error output

2018-12-17 Thread redi at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88512

--- Comment #7 from Jonathan Wakely  ---
I don't think this has anything to do with the std::lib anyway (and certainly
not "the STL" which the example doesn't use at all). Most of the differences
between GCC and Clang can be shown by an example like this with no std::lib
types:

template
struct basic_string {
  using iterator = C*;
  using const_iterator = const C*;
  using size_type = unsigned long;

  iterator end();
  size_type find(C) const;
  void erase(size_type, size_type);
  void erase(const_iterator);
  void erase(const_iterator, const_iterator);
};
using string = basic_string;

int main()
{
string str;
auto s = str.find(' ');
str.erase(s, str.end());
}

I see two concrete differences.

Firstly, GCC says:

s.cc:19:27: error: no matching function for call to 'erase(long unsigned int&,
basic_string::iterator)'

vs Clang's

s.cc:19:9: error: no matching member function for call to 'erase'

Arguably, Clang's is better. We're showing a member function signature for a
member that doesn't exist, which isn't very helpful. THe original is even
worse, showing the made up member as a qualified name:

s.cc:6:27: error: no matching function for call to
'std::basic_string::erase(std::size_t&,
std::basic_string::iterator)'

(I don't know why that shows std::__cxx11::basic_string::erase(...) and my
reduced example only shows erase(...), but showing it as a member is worse
IMHO).

The fact that the arguments have type size_t& and string::iterator doesn't
imply there's a member function with parameters of those type. Those arguments
could match:

erase(size_t, const_iterator)
erase(int, const_iterator);

or any other number of functions callable with size_t lvalue and iterator
rvalue.

So displaying some made up member function as a means of showing the argument
types is not great. On the other hand, sometimes that first error is all you
need to look at, because it might be obvious from 'erase(size_t&, iterator)'
that you mixed up the arguments. In that case you can ignore the errors showing
the overload resolution results. Clang's brief 'erase' doesn't help with that,
you have to read the later errors.

Secondly, for each overload candidate Clang shows a single note that says why
overload resolution failed (and the declaration of the candidate with a caret
location):

s.cc:9:8: note: candidate function not viable: no known conversion from
'basic_string::iterator' (aka 'char *') to 'basic_string::size_type' (aka
'unsigned long') for 2nd argument; dereference the argument with *
  void erase(size_type, size_type);
   ^

GCC shows a note describing the candidate (with a caret location for the
declaration of that candidate), and a second note saying why overload
resolution failed, and the failure reason as an error (and a second caret
location showing the location of that error):

s.cc:9:8: note: candidate: 'void
basic_string::erase(basic_string::size_type, basic_string::size_type)
[with C = char; basic_string::size_type = long unsigned int]' 
9 |   void erase(size_type, size_type);
  |^
s.cc:9:8: note:   conversion of argument 2 would be ill-formed:
s.cc:19:25: error: invalid conversion from 'basic_string::iterator' {aka
'char*'} to 'basic_string::size_type' {aka 'long unsigned int'}
[-fpermissive]
   19 | str.erase(s, str.end());
  |  ~~~^~
  | |
  | basic_string::iterator {aka char*}

So Clang has a note and a caret location, but GCC has two notes, an error, and
two caret locations. But it's not clear to me that any of GCC's output can be
removed without losing useful information. Clang shows less output, but that
also includes less information.

The only change I'd make that seems definitely an improvement would be to drop
the class name qualification from the made up function signature here:

stl_string.cpp:7:27: error: no matching function for call to
‘std::__cxx11::basic_string::erase(std::size_t&,
std::__cxx11::basic_string::iterator)’


Additionally, we should use the standard typedefs and display
std::__cxx11::basic_string<_CharT, _Traits, _Alloc> as std::string (I think
there's an existing bug for that, though I can't find it).

[Bug c++/88512] Too much STL in error output

2018-12-17 Thread redi at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88512

--- Comment #6 from Jonathan Wakely  ---
(In reply to Jonny Grant from comment #3)
> Hi Marc
> 
> I agree to useful to have the option to keep on for regular code.
> Perhaps a way just to turn off all expansive output for STL's std namespace?

But what about cases where you call a std::lib function template that accepts
any arguments, which forwards them to some other std::lib function that gives
an error because the args are invalid. If you remove all the context that's in
the std::lib then you remove the relevant context.


> The easiest would be to remove cxx11 namespace from the messages
> "std::__cxx11::basic_string" etc, so at least that doesn't shwo.
> 
> 
> Updated output I had executed from STL with clear candidate suggestions:
> 
> $ g++ -Wall -o stl_string stl_string.cpp
> stl_string.cpp: In function ‘int main()’:
> stl_string.cpp:7:27: error: no matching function for call to
> ‘std::string::erase(std::size_t&, std::string::iterator)’
>  str.erase(s, str.end());
>  ^
> candidates are:
> std::string& std::string::erase(size_type index = 0, size_type count =
> npos)
> std::string::iterator std::string::erase(const_iterator first,
> const_iterator last)
> std::string::iterator std::string::erase(const_iterator position)

You've removed all the information that says *why* it didn't match (e.g. no
conversion from size_t to const_iterator). That's useful.

You don't improve the compiler's diagnostics by optimising for a single example
where the verbose info happens to be unnecessary. That info is useful in other
cases.

[Bug c++/88512] Too much STL in error output

2018-12-16 Thread jg at jguk dot org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88512

--- Comment #5 from Jonny Grant  ---
ICC 19.0.1 output

(7): error: no instance of overloaded function
"std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::erase [with _CharT=char,
_Traits=std::char_traits, _Alloc=std::allocator]" matches the
argument list

argument types are: (std::size_t, __gnu_cxx::__normal_iterator,
std::allocator>>)

object type is: std::__cxx11::string

  str.erase(s, str.end());
  ^

/opt/compiler-explorer/gcc-8.2.0/bin/../include/c++/8.2.0/bits/basic_string.h(1827):
note: this candidate was rejected because arguments do not match

erase(__const_iterator __first, __const_iterator __last)
^

/opt/compiler-explorer/gcc-8.2.0/bin/../include/c++/8.2.0/bits/basic_string.h(1808):
note: this candidate was rejected because mismatch in count of arguments

erase(__const_iterator __position)
^

/opt/compiler-explorer/gcc-8.2.0/bin/../include/c++/8.2.0/bits/basic_string.h(1789):
note: this candidate was rejected because arguments do not match

erase(size_type __pos = 0, size_type __n = npos)
^

compilation aborted for  (code 2)
Compiler returned: 2

[Bug c++/88512] Too much STL in error output

2018-12-15 Thread glisse at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88512

--- Comment #4 from Marc Glisse  ---
(In reply to Jonny Grant from comment #3)
> Updated output I had executed from STL with clear candidate suggestions:
> 
> $ g++ -Wall -o stl_string stl_string.cpp
> stl_string.cpp: In function ‘int main()’:
> stl_string.cpp:7:27: error: no matching function for call to
> ‘std::string::erase(std::size_t&, std::string::iterator)’
>  str.erase(s, str.end());
>  ^
> candidates are:
> std::string& std::string::erase(size_type index = 0, size_type count =
> npos)
> std::string::iterator std::string::erase(const_iterator first,
> const_iterator last)
> std::string::iterator std::string::erase(const_iterator position)

Maybe for the standard library... For user code I would still want a reminder
somewhere of what size_type, iterator, const_iterator and string are aliases
for. Besides, gcc currently tells you why the candidate was not picked, which
is also useful.
What I expect would work is to have the message as a tree that originally only
shows the error message, but where you can click to show the candidates, click
a candidate to show why it fails, click a type to see what it aliases, etc. But
that's only doable through an IDE.

[Bug c++/88512] Too much STL in error output

2018-12-15 Thread jg at jguk dot org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88512

--- Comment #3 from Jonny Grant  ---
Hi Marc

I agree to useful to have the option to keep on for regular code.
Perhaps a way just to turn off all expansive output for STL's std namespace?

The easiest would be to remove cxx11 namespace from the messages
"std::__cxx11::basic_string" etc, so at least that doesn't shwo.


Updated output I had executed from STL with clear candidate suggestions:

$ g++ -Wall -o stl_string stl_string.cpp
stl_string.cpp: In function ‘int main()’:
stl_string.cpp:7:27: error: no matching function for call to
‘std::string::erase(std::size_t&, std::string::iterator)’
 str.erase(s, str.end());
 ^
candidates are:
std::string& std::string::erase(size_type index = 0, size_type count =
npos)
std::string::iterator std::string::erase(const_iterator first,
const_iterator last)
std::string::iterator std::string::erase(const_iterator position)

[Bug c++/88512] Too much STL in error output

2018-12-15 Thread glisse at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88512

--- Comment #2 from Marc Glisse  ---
As someone who regularly has to debug complicated template code, I think all
the information printed by g++ here is useful. There may be more compact ways
of giving the same information (use a shorthand for
"std::__cxx11::basic_string<_CharT, _Traits, _Alloc>"?), but I do want all the
information.

We could special case standard library code to print less details because
supposedly the library is perfect, we are debugging user code and not the
library, so some of the information is unnecessary, but I am not convinced this
is doable.

[Bug c++/88512] Too much STL in error output

2018-12-15 Thread jg at jguk dot org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88512

--- Comment #1 from Jonny Grant  ---
MSVC 19 isn't much better

(7): error C2664:
'std::_String_iterator>>
std::basic_string,std::allocator<_Ty>>::erase(const
std::_String_const_iterator>>,const
std::_String_const_iterator>>)':
cannot convert argument 2 from
'std::_String_iterator>>' to 'unsigned
__int64'

with

[

_Ty=char

]

(7): note: No user-defined-conversion operator available that can
perform this conversion, or the operator cannot be called

Compiler returned: 2