On Tue, 9 Apr 2024, Patrick Palka wrote:
> On Thu, 19 Oct 2023, Patrick Palka wrote:
>
> > On Tue, 3 Oct 2023, David Malcolm wrote:
> >
> > > As mentioned in my Cauldron talk, this patch adds a call to
> > > diagnostic_show_locus to the "required from here" messages
> > > in print_instantiation_partial_context_line, so that e.g., rather
> > > than the rather mystifying:
> > >
> > > In file included from
> > > ../x86_64-pc-linux-gnu/libstdc++-v3/include/memory:78,
> > > from ../../src/demo-1.C:1:
> > > ../x86_64-pc-linux-gnu/libstdc++-v3/include/bits/unique_ptr.h: In
> > > instantiation of ‘std::__detail::__unique_ptr_t<_Tp>
> > > std::make_unique(_Args&& ...) [with _Tp = bar; _Args = {};
> > > __detail::__unique_ptr_t<_Tp> = __detail::__unique_ptr_t<bar>]’:
> > > ../../src/demo-1.C:15:32: required from here
> > > ../x86_64-pc-linux-gnu/libstdc++-v3/include/bits/unique_ptr.h:1066:30:
> > > error: no matching function for call to ‘bar::bar()’
> > > 1066 | { return unique_ptr<_Tp>(new
> > > _Tp(std::forward<_Args>(__args)...)); }
> > > |
> > > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > ../../src/demo-1.C:10:3: note: candidate: ‘bar::bar(int)’
> > > 10 | bar (int);
> > > | ^~~
> > > ../../src/demo-1.C:10:3: note: candidate expects 1 argument, 0 provided
> > > ../../src/demo-1.C:7:7: note: candidate: ‘constexpr bar::bar(const bar&)’
> > > 7 | class bar : public foo
> > > | ^~~
> > > ../../src/demo-1.C:7:7: note: candidate expects 1 argument, 0 provided
> > > ../../src/demo-1.C:7:7: note: candidate: ‘constexpr bar::bar(bar&&)’
> > > ../../src/demo-1.C:7:7: note: candidate expects 1 argument, 0 provided
> > >
> > > we emit:
> > >
> > > In file included from
> > > ../x86_64-pc-linux-gnu/libstdc++-v3/include/memory:78,
> > > from ../../src/demo-1.C:1:
> > > ../x86_64-pc-linux-gnu/libstdc++-v3/include/bits/unique_ptr.h: In
> > > instantiation of ‘std::__detail::__unique_ptr_t<_Tp>
> > > std::make_unique(_Args&& ...) [with _Tp = bar; _Args = {};
> > > __detail::__unique_ptr_t<_Tp> = __detail::__unique_ptr_t<bar>]’:
> > > ../../src/demo-1.C:15:32: required from here
> > > 15 | return std::make_unique<bar> ();
> > > | ~~~~~~~~~~~~~~~~~~~~~~^~
> > > ../x86_64-pc-linux-gnu/libstdc++-v3/include/bits/unique_ptr.h:1066:30:
> > > error: no matching function for call to ‘bar::bar()’
> > > 1066 | { return unique_ptr<_Tp>(new
> > > _Tp(std::forward<_Args>(__args)...)); }
> > > |
> > > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > ../../src/demo-1.C:10:3: note: candidate: ‘bar::bar(int)’
> > > 10 | bar (int);
> > > | ^~~
> > > ../../src/demo-1.C:10:3: note: candidate expects 1 argument, 0 provided
> > > ../../src/demo-1.C:7:7: note: candidate: ‘constexpr bar::bar(const bar&)’
> > > 7 | class bar : public foo
> > > | ^~~
> > > ../../src/demo-1.C:7:7: note: candidate expects 1 argument, 0 provided
> > > ../../src/demo-1.C:7:7: note: candidate: ‘constexpr bar::bar(bar&&)’
> > > ../../src/demo-1.C:7:7: note: candidate expects 1 argument, 0 provided
> > >
> > > which shows the code that's leading to the error (the bad call to
> > > std::make_unique).
> >
> > This is great! I noticed however that the source code gets printed in a
> > surprising way in some contexts. Consider:
> >
> > template<class T> void f(typename T::type);
> >
> > int main() {
> > f<int>(0);
> > }
> >
> > For this testcase we emit:
> >
> > testcase.C: In function ‘int main()’:
> > testcase.C:4:9: error: no matching function for call to ‘f<int>(int)’
> > 4 | f<int>(0);
> > | ~~~~~~^~~
> > testcase.C:1:24: note: candidate: ‘template<class T> void f(typename
> > T::type)’
> > 1 | template<class T> void f(typename T::type);
> > | ^
> > testcase.C:1:24: note: template argument deduction/substitution failed:
> > testcase.C: In substitution of ‘template<class T> void f(typename T::type)
> > [with T = int]’:
> > testcase.C:4:9: required from here
> > testcase.C:1:24: note: 4 | f<int>(0);
> > testcase.C:1:24: note: | ~~~~~~^~~
> > testcase.C:1:24: error: ‘int’ is not a class, struct, or union type
> > 1 | template<class T> void f(typename T::type);
> > | ^
> >
> > In particular the source code part following the "required from here" line
> >
> > testcase.C:4:9: required from here
> > testcase.C:1:24: note: 4 | f<int>(0);
> > testcase.C:1:24: note: | ~~~~~~^~~
> >
> > seems off, I would have expected it be
> >
> > testcase.C:4:9: required from here
> > 4 | f<int>(0);
> > | ~~~~~~^~~
> >
> > i.e. without the "testcase.C:1:24: note: " prefix. Does this look
> > expected? (I also wonder if we might want to omit printing the source
> > code altogether in this case, since we already printed that same line
> > earlier during the "no matching function" error?)
>
> Sorry for forgetting to ping this earlier, but I wonder what if anything
> we should do about this for GCC 14?
Here's another example of the source code rendering issue using a test
from the libstdc++ testsuite:
$ g++ -D_GLIBCXX_DEBUG -Wsystem-headers -Wsign-compare -std=c++23 -I
libstdc++-v3/testsuite/util
libstdc++-v3/testsuite/std/ranges/adaptors/enumerate/1.cc
In file included from
/scratchpad/gcc-build/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/version.h:49,
from
/scratchpad/gcc-build/x86_64-pc-linux-gnu/libstdc++-v3/include/concepts:36,
from
/scratchpad/gcc-build/x86_64-pc-linux-gnu/libstdc++-v3/include/ranges:37,
from
libstdc++-v3/testsuite/std/ranges/adaptors/enumerate/1.cc:4:
/scratchpad/gcc-build/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/basic_string.tcc:
In instantiation of ‘constexpr void std::__cxx11::basic_string<_CharT,
_Traits, _Alloc>::resize_and_overwrite(size_type, _Operation) [with _Operation
= std::__format::__formatter_fp<wchar_t>::format<float,
std::__format::_Sink_iter<wchar_t> >(float,
std::basic_format_context<std::__format::_Sink_iter<wchar_t>, wchar_t>&)
const::<lambda(char*, std::size_t)>&; _CharT = char; _Traits =
std::char_traits<char>; _Alloc = std::allocator<char>; size_type = long
unsigned int]’:
[ ... snip ... ]
/scratchpad/gcc-build/x86_64-pc-linux-gnu/libstdc++-v3/include/format:4038:23:
required from ‘constexpr void std::__format::_Formatting_scanner<_Out,
_CharT>::_M_format_arg(std::size_t) [with _Out =
std::__format::_Sink_iter<wchar_t>; _CharT = wchar_t; std::size_t = long
unsigned int]’
4038 | std::visit_format_arg([this](auto& __arg) {
| ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~
4039 | using _Type = remove_reference_t<decltype(__arg)>;
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
4040 | using _Formatter = typename _Context::template
formatter_type<_Type>;
|
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
4041 | if constexpr (is_same_v<_Type, monostate>)
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
4042 | __format::__invalid_arg_id_in_format_string();
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
4043 | else if constexpr (is_same_v<_Type, handle>)
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
4044 | __arg.format(this->_M_pc, this->_M_fc);
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
4045 | else if constexpr (is_default_constructible_v<_Formatter>)
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
4046 | {
| ~
4047 | _Formatter __f;
| ~~~~~~~~~~~~~~~
4048 | this->_M_pc.advance_to(__f.parse(this->_M_pc));
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
4049 | this->_M_fc.advance_to(__f.format(__arg, this->_M_fc));
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
4050 | }
| ~
4051 | else
| ~~~~
4052 | static_assert(__format::__formattable_with<_Type,
_Context>);
|
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
4053 | }, _M_fc.arg(__id));
| ~~~~~~~~~~~~~~~~~~~
/scratchpad/gcc-build/x86_64-pc-linux-gnu/libstdc++-v3/include/format:4033:7:
required from here
4033 | _M_format_arg(size_t __id) override
| ^~~~~~~~~~~~~
/scratchpad/gcc-build/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/basic_string.tcc:609:7:
warning: comparison of integer expressions of different signedness: ‘long int’
and ‘const std::__cxx11::basic_string<char>::size_type’ {aka ‘const long
unsigned int’} [-Wsign-compare]
609 | _GLIBCXX_DEBUG_ASSERT(__r >= 0 && __r <= __n);
| ^~~~~~~~~~~~~~~~~~~~~
/scratchpad/gcc-build/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/basic_string.tcc:
In instantiation of ‘constexpr void std::__cxx11::basic_string<_CharT,
_Traits, _Alloc>::resize_and_overwrite(size_type, _Operation) [with _Operation
= std::__format::__formatter_fp<wchar_t>::format<double,
std::__format::_Sink_iter<wchar_t> >(double,
std::basic_format_context<std::__format::_Sink_iter<wchar_t>, wchar_t>&)
const::<lambda(char*, std::size_t)>&; _CharT = char; _Traits =
std::char_traits<char>; _Alloc = std::allocator<char>; size_type = long
unsigned int]’:
[ ... snip ... ]
/scratchpad/gcc-build/x86_64-pc-linux-gnu/libstdc++-v3/include/format:4038:23:
required from ‘constexpr void std::__format::_Formatting_scanner<_Out,
_CharT>::_M_format_arg(std::size_t) [with _Out =
std::__format::_Sink_iter<wchar_t>; _CharT = wchar_t; std::size_t = long
unsigned int]’
/scratchpad/gcc-build/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/basic_string.tcc:609:7:
warning: 4038 | std::visit_format_arg([this](auto& __arg) {
/scratchpad/gcc-build/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/basic_string.tcc:609:7:
warning: | ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~
4039 | using _Type = remove_reference_t<decltype(__arg)>;
/scratchpad/gcc-build/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/basic_string.tcc:609:7:
warning: | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
4040 | using _Formatter = typename _Context::template
formatter_type<_Type>;
/scratchpad/gcc-build/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/basic_string.tcc:609:7:
warning: |
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
4041 | if constexpr (is_same_v<_Type, monostate>)
/scratchpad/gcc-build/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/basic_string.tcc:609:7:
warning: | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
4042 | __format::__invalid_arg_id_in_format_string();
/scratchpad/gcc-build/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/basic_string.tcc:609:7:
warning: | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
4043 | else if constexpr (is_same_v<_Type, handle>)
/scratchpad/gcc-build/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/basic_string.tcc:609:7:
warning: | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
4044 | __arg.format(this->_M_pc, this->_M_fc);
/scratchpad/gcc-build/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/basic_string.tcc:609:7:
warning: | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
4045 | else if constexpr (is_default_constructible_v<_Formatter>)
/scratchpad/gcc-build/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/basic_string.tcc:609:7:
warning: |
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
4046 | {
/scratchpad/gcc-build/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/basic_string.tcc:609:7:
warning: | ~
4047 | _Formatter __f;
/scratchpad/gcc-build/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/basic_string.tcc:609:7:
warning: | ~~~~~~~~~~~~~~~
4048 | this->_M_pc.advance_to(__f.parse(this->_M_pc));
/scratchpad/gcc-build/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/basic_string.tcc:609:7:
warning: | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
4049 | this->_M_fc.advance_to(__f.format(__arg, this->_M_fc));
/scratchpad/gcc-build/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/basic_string.tcc:609:7:
warning: |
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
4050 | }
/scratchpad/gcc-build/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/basic_string.tcc:609:7:
warning: | ~
4051 | else
/scratchpad/gcc-build/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/basic_string.tcc:609:7:
warning: | ~~~~
4052 | static_assert(__format::__formattable_with<_Type,
_Context>);
/scratchpad/gcc-build/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/basic_string.tcc:609:7:
warning: |
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
4053 | }, _M_fc.arg(__id));
/scratchpad/gcc-build/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/basic_string.tcc:609:7:
warning: | ~~~~~~~~~~~~~~~~~~~
/scratchpad/gcc-build/x86_64-pc-linux-gnu/libstdc++-v3/include/format:4033:7:
required from here
/scratchpad/gcc-build/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/basic_string.tcc:609:7:
warning: 4033 | _M_format_arg(size_t __id) override
/scratchpad/gcc-build/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/basic_string.tcc:609:7:
warning: | ^~~~~~~~~~~~~
/scratchpad/gcc-build/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/basic_string.tcc:609:7:
warning: comparison of integer expressions of different signedness: ‘long int’
and ‘const std::__cxx11::basic_string<char>::size_type’ {aka ‘const long
unsigned int’} [-Wsign-compare]
609 | _GLIBCXX_DEBUG_ASSERT(__r >= 0 && __r <= __n);
| ^~~~~~~~~~~~~~~~~~~~~
[ ... snip ... ]
The instantiation context source code for the first warning get rendered fine,
but for subsequent warnings they contain a spurious "file:line:col: warning: "
prefix which seems to be inherited from the warning before it.
>
> >
> > >
> > >
> > > Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
> > >
> > > OK for trunk?
> > >
> > >
> > > gcc/cp/ChangeLog:
> > > * error.cc (print_instantiation_partial_context_line): Call
> > > diagnostic_show_locus.
> > >
> > > gcc/testsuite/ChangeLog:
> > > * g++.dg/diagnostic/static_assert3.C: Add directives for
> > > additional source printing.
> > > * g++.dg/template/error60.C: New test.
> > >
> > > Signed-off-by: David Malcolm <dmalc...@redhat.com>
> > > ---
> > > gcc/cp/error.cc | 2 +
> > > .../g++.dg/diagnostic/static_assert3.C | 7 +++-
> > > gcc/testsuite/g++.dg/template/error60.C | 37 +++++++++++++++++++
> > > 3 files changed, 45 insertions(+), 1 deletion(-)
> > > create mode 100644 gcc/testsuite/g++.dg/template/error60.C
> > >
> > > diff --git a/gcc/cp/error.cc b/gcc/cp/error.cc
> > > index ef96e140f24..767478cf5fd 100644
> > > --- a/gcc/cp/error.cc
> > > +++ b/gcc/cp/error.cc
> > > @@ -3774,6 +3774,8 @@ print_instantiation_partial_context_line
> > > (diagnostic_context *context,
> > > ? _("recursively required from here\n")
> > > : _("required from here\n"));
> > > }
> > > + gcc_rich_location rich_loc (loc);
> > > + diagnostic_show_locus (context, &rich_loc, DK_NOTE);
> > > }
> > >
> > > /* Same as print_instantiation_full_context but less verbose. */
> > > diff --git a/gcc/testsuite/g++.dg/diagnostic/static_assert3.C
> > > b/gcc/testsuite/g++.dg/diagnostic/static_assert3.C
> > > index 5d363884508..4ec53f17120 100644
> > > --- a/gcc/testsuite/g++.dg/diagnostic/static_assert3.C
> > > +++ b/gcc/testsuite/g++.dg/diagnostic/static_assert3.C
> > > @@ -5,6 +5,11 @@
> > > template <typename T, typename U> struct is_same { static constexpr bool
> > > value = false; };
> > > template <typename T> struct is_same<T, T> { static constexpr bool value
> > > = true; };
> > >
> > > +/* { dg-begin-multiline-output "" }
> > > + f(0, 1.3);
> > > + ~^~~~~~~~
> > > + { dg-end-multiline-output "" } */
> > > +
> > > template <typename T, typename U>
> > > void f(T, U)
> > > {
> > > @@ -32,5 +37,5 @@ void f(T, U)
> > >
> > > void g()
> > > {
> > > - f(0, 1.3);
> > > + f(0, 1.3); // { dg-message " required from here" }
> > > }
> > > diff --git a/gcc/testsuite/g++.dg/template/error60.C
> > > b/gcc/testsuite/g++.dg/template/error60.C
> > > new file mode 100644
> > > index 00000000000..8c2139b207c
> > > --- /dev/null
> > > +++ b/gcc/testsuite/g++.dg/template/error60.C
> > > @@ -0,0 +1,37 @@
> > > +// { dg-options "-fdiagnostics-show-caret" }
> > > +
> > > +template <typename Foo>
> > > +struct my_pointer
> > > +{
> > > + my_pointer (Foo *ptr) // { dg-message " initializing argument 1" }
> > > + : m_ptr (ptr)
> > > + {}
> > > +
> > > + Foo *m_ptr;
> > > +};
> > > +
> > > +template <typename Foo>
> > > +void test (Foo val)
> > > +{
> > > + my_pointer<Foo> ptr (val); // { dg-error "invalid conversion from
> > > 'int' to 'int\\*'" }
> > > +}
> > > +
> > > +void usage ()
> > > +{
> > > + test<int> (42); // { dg-message " required from here" }
> > > + /* { dg-begin-multiline-output "" }
> > > + test<int> (42);
> > > + ~~~~~~~~~~^~~~
> > > + { dg-end-multiline-output "" } */
> > > +}
> > > +
> > > + /* { dg-begin-multiline-output "" }
> > > + my_pointer (Foo *ptr)
> > > + ~~~~~^~~
> > > + { dg-end-multiline-output "" } */
> > > + /* { dg-begin-multiline-output "" }
> > > + my_pointer<Foo> ptr (val);
> > > + ^~~
> > > + |
> > > + int
> > > + { dg-end-multiline-output "" } */
> > > --
> > > 2.26.3
> > >
> > >
