Re: [PATCH 2/5] c++: Set the locus of the function result decl

[Bernhard Reutner-Fischer <rep.dot.nop@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 5380 bytes --]

On Thu, 17 Nov 2022 18:52:36 -0500
Jason Merrill <jason@redhat.com> wrote:

> On 11/17/22 14:02, Bernhard Reutner-Fischer wrote:
> > On Thu, 17 Nov 2022 09:53:32 -0500
> > Jason Merrill <jason@redhat.com> wrote:

> >> Instead, you want to copy the location for instantiations, i.e. check
> >> DECL_TEMPLATE_INSTANTIATION instead of !DECL_USE_TEMPLATE.  
> > 
> > No, that makes no difference.  
> 
> Hmm, when I stop there when processing the instantiation the template's 
> DECL_RESULT has the right location information, e.g. for
> 
> template <class T> int f() { return 42; }
> 
> int main()
> {
>    f<int>();
> }
> 
> #1  0x0000000000f950e8 in instantiate_body (pattern=<template_decl 
> 0x7ffff7ff5080 f>, args=<tree_vec 0x7fffe9712ae0>, d=<function_decl 
> 0x7fffe971e600 f>, nested_p=false) at /home/jason/gt/gcc/cp/pt.cc:26470
> #0  start_preparsed_function (decl1=<function_decl 0x7fffe971e600 f>, 
> attrs=<tree 0x0>, flags=1) at /home/jason/gt/gcc/cp/decl.cc:17252
> (gdb) p expand_location (input_location)
> $13 = {file = 0x4962370 "wa.C", line = 1, column = 24, data = 0x0, sysp 
> = false}
> (gdb) p expand_location (DECL_SOURCE_LOCATION (DECL_RESULT 
> (DECL_TEMPLATE_RESULT (DECL_TI_TEMPLATE (decl1)))))
> $14 = {file = 0x4962370 "wa.C", line = 1, column = 20, data = 0x0, sysp 
> = false}

Yes, that works. Sorry if i was not clear: The thing in the cover
letter in this series does not work, the mini_vector reduced testcase
from the libstdc++-v3/include/ext/bitmap_allocator.h.
class template member function return type location, would that be it?

AFAIR the problem was that that these member functions get their result
decl late. When they get them, there are no
declspecs->locations[ds_type_spec] around anywhere to tuck that on the
resdecl. While the result decl is clear, there is no obvious way where
to store the ds_type_spec (somewhere in the template, as you told me).

Back then I tried moving the resdecl building from
start_preparsed_function to grokfndecl but that did not work out easily
IIRC and i ultimately gave up to move stuff around rather blindly.
I also tried to find a spot where i could store the ds_type_spec locus
somewhere in grokmethod, but i think the problem was the same, i had
just the type where i cannot store a locus and did not find a place
where i could smuggle the locus along.

So, to make that clear. Your template function (?) works:

$ XXX=1 ./xg++ -B. -S -o /dev/null ../tmp4/return-narrow-2j.cc 
../tmp4/return-narrow-2j.cc: In function ‘int f()’:
../tmp4/return-narrow-2j.cc:1:20: warning: result decl locus sample
    1 | template <class T> int f() { return 42; }
      |                    ^~~
      |                    the return type
../tmp4/return-narrow-2j.cc: In function ‘int main()’:
../tmp4/return-narrow-2j.cc:3:1: warning: result decl locus sample
    3 | int main()
      | ^~~
      | the return type
../tmp4/return-narrow-2j.cc: In instantiation of ‘int f() [with T = int]’:
../tmp4/return-narrow-2j.cc:5:10:   required from here
../tmp4/return-narrow-2j.cc:1:20: warning: result decl locus sample
    1 | template <class T> int f() { return 42; }
      |                    ^~~
      |                    the return type


The class member fn not so much (IMHO, see attached):

$ XXX=1 ./xg++ -B. -S -o /dev/null ../tmp4/return-narrow-2.cc 
../tmp4/return-narrow-2.cc: In member function ‘const long unsigned int __mini_vector< <template-parameter-1-1> >::_M_space_left()’:
../tmp4/return-narrow-2.cc:9:3: warning: result decl locus sample
    9 |   { return _M_finish != 0; }
      |   ^
      |   the return type
../tmp4/return-narrow-2.cc: In instantiation of ‘const long unsigned int __mini_vector< <template-parameter-1-1> >::_M_space_left() [with <template-parameter-1-1> = std::pair<long int, long int>]’:
../tmp4/return-narrow-2.cc:11:17:   required from here
../tmp4/return-narrow-2.cc:9:3: warning: result decl locus sample
    9 |   { return _M_finish != 0; }
      |   ^
      |   the return type
../tmp4/return-narrow-2.cc: In instantiation of ‘const long unsigned int __mini_vector< <template-parameter-1-1> >::_M_space_left() [with <template-parameter-1-1> = int]’:
../tmp4/return-narrow-2.cc:12:17:   required from here
../tmp4/return-narrow-2.cc:9:3: warning: result decl locus sample
    9 |   { return _M_finish != 0; }
      |   ^
      |   the return type


> 
> > But really I'm not interested in the template case, i only mentioned
> > them because they don't work and in case somebody wanted to have correct
> > locations.
> > I remember just frustration when i looked at those a year ago.  
> 
> I'd like to get the template case right while we're looking at it.  I 
> guess I can add that myself if you're done trying.
> 
> > Is the hunk for normal functions OK for trunk?  
> 
> You also need a testcase for the desired behavior, with e.g.
> { dg-error "23:" }

I'd have to think about how to test that with trunk, yes.
There are no existing warnings that want to point to the return type,
are there?

Maybe a g++.dg/plugin/result_decl_plugin.c then.

set plugin_test_list [list
hmz. That strikes me as not all that flexible.
We could glob *_plugin.[cC][c]*, and have foo_plugin.lst contain it's
files. Whatever.

thanks,

[-- Attachment #2: c++-ctmfrv-locus3.patch --]
[-- Type: text/plain, Size: 2386 bytes --]

diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
index d28889ed865..e0f057fc37b 100644
--- a/gcc/cp/decl.cc
+++ b/gcc/cp/decl.cc
@@ -17235,6 +17236,17 @@ start_preparsed_function (tree decl1, tree attrs, int flags)
       cp_apply_type_quals_to_decl (cp_type_quals (restype), resdecl);
     }
 
+  /* Set the result decl source location to the location of the typespec.  */
+  if (DECL_RESULT (decl1)
+      && DECL_TEMPLATE_INSTANTIATION (decl1)
+      && DECL_TEMPLATE_INFO (decl1)
+      && DECL_TI_TEMPLATE (decl1)
+      && DECL_TEMPLATE_RESULT (DECL_TI_TEMPLATE (decl1))
+      && DECL_RESULT (DECL_TEMPLATE_RESULT (DECL_TI_TEMPLATE (decl1))))
+      DECL_SOURCE_LOCATION (DECL_RESULT (decl1))
+	= DECL_SOURCE_LOCATION (
+	    DECL_RESULT (DECL_TEMPLATE_RESULT (DECL_TI_TEMPLATE (decl1))));
+
   /* Record the decl so that the function name is defined.
      If we already have a decl for this name, and it is a FUNCTION_DECL,
      use the old decl.  */
@@ -17532,7 +17544,19 @@ start_function (cp_decl_specifier_seq *declspecs,
     gcc_assert (same_type_p (TREE_TYPE (TREE_TYPE (decl1)),
 			     integer_type_node));
 
-  return start_preparsed_function (decl1, attrs, /*flags=*/SF_DEFAULT);
+  bool ret = start_preparsed_function (decl1, attrs, /*flags=*/SF_DEFAULT);
+
+  /* decl1 might be ggc_freed here.  */
+  decl1 = current_function_decl;
+
+  tree result;
+  /* Set the result decl source location to the location of the typespec.  */
+  if (TREE_CODE (decl1) == FUNCTION_DECL
+      && declspecs->locations[ds_type_spec] != UNKNOWN_LOCATION
+      && (result = DECL_RESULT (decl1)) != NULL_TREE
+      && DECL_SOURCE_LOCATION (result) == input_location)
+    DECL_SOURCE_LOCATION (result) = declspecs->locations[ds_type_spec];
+  return ret;
 }
 \f
 /* Returns true iff an EH_SPEC_BLOCK should be created in the body of
@@ -18063,6 +18087,14 @@ finish_function (bool inline_p)
 	suppress_warning (fndecl, OPT_Wreturn_type);
     }
 
+    if (getenv("XXX") != NULL)
+      {
+	location_t result_loc = DECL_SOURCE_LOCATION (DECL_RESULT (fndecl));
+	gcc_rich_location richloc (result_loc);
+	richloc.add_fixit_replace (result_loc, "the return type");
+	warning_at (&richloc, 0, "result dec%c locus sample", 'l');
+      }
+
   /* Lambda closure members are implicitly constexpr if possible.  */
   if (cxx_dialect >= cxx17
       && LAMBDA_TYPE_P (CP_DECL_CONTEXT (fndecl)))

[-- Attachment #3: return-narrow-2.cc --]
[-- Type: text/plain, Size: 325 bytes --]

namespace std { template < typename, typename > struct pair; }
template < typename > struct __mini_vector
{
  int _M_finish;
  const
  unsigned long
  __attribute__((deprecated))
  _M_space_left()
  { return _M_finish != 0; }
};
 template class __mini_vector< std::pair< long, long > >;
 template class __mini_vector< int >;