On Wed, 24 Apr 2024, Patrick Palka wrote:
> 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.
I'm testing the following which seems to fix it:
diff --git a/gcc/cp/error.cc b/gcc/cp/error.cc
index 7074845154e..f38a0d9906a 100644
--- a/gcc/cp/error.cc
+++ b/gcc/cp/error.cc
@@ -3793,7 +3793,10 @@ print_instantiation_partial_context_line
(diagnostic_context *context,
: _("required from here\n"));
}
gcc_rich_location rich_loc (loc);
+ char *saved_prefix = pp_take_prefix (context->printer);
+ pp_set_prefix (context->printer, NULL);
diagnostic_show_locus (context, &rich_loc, DK_NOTE);
+ pp_set_prefix (context->printer, saved_prefix);
}
/* Same as print_instantiation_full_context but less verbose. */
>
> >
> > >
> > > >
> > > >
> > > > 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
> > > >
> > > >
