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?
>
> >
> >
> > 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
> >
> >
