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

--- Comment #7 from Jonathan Wakely <redi at gcc dot gnu.org> ---
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<typename C>
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<char>;

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<char>::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<char>::erase(std::size_t&,
std::basic_string<char>::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<char>::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<C>::erase(basic_string<C>::size_type, basic_string<C>::size_type)
[with C = char; basic_string<C>::size_type = long unsigned int]' <near match>
    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<char>::iterator' {aka
'char*'} to 'basic_string<char>::size_type' {aka 'long unsigned int'}
[-fpermissive]
   19 |     str.erase(s, str.end());
      |                  ~~~~~~~^~
      |                         |
      |                         basic_string<char>::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<char>::erase(std::size_t&,
std::__cxx11::basic_string<char>::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).

Reply via email to