mbox series

[0/5] function result decl location; type demotion

Message ID 20221112234543.95441-1-aldot@gcc.gnu.org
Headers show
Series function result decl location; type demotion | expand

Message

Bernhard Reutner-Fischer Nov. 12, 2022, 11:45 p.m. UTC
Hi!

The location of function result declarations was not set.
The first two patches set the location of normal functions in C and C++.

Jason, Nathan, I failed to support C++ template functions, see below.

TL;DR.
Why all this?
PR78798 noted that we should use narrower function return types if feasable.

So one idea to bring that idea forward is to determine the range a
function can return and match that range against the range provided by
the actual return type. If we waste bits, warn and, ideally, suggest a
better alternative. Ideally with a fix-it hint for the narrower type and
with a patch.
David's tremendously useful work on diagnostics makes both user-facing
aspecs rather easy to achieve (thanks, once again, David!).
Ideally, one would be able to accumulate such suggested fix-it hints
driven patches by stating something like:
  ... -Wtype-demotion -fdiagnostics-generate-patch=>>/tmp/hmz.patch
i.e. have ways to direct the ...-generate-patch to creat/append to some
given path. But that doesn't seem to work for me or i did not read the
documentation carefully enough. awk to the rescue for the full buildlog
output to extract the patch(es) but not all that userfriendly i fear.
How would i write a combined patch out to some given path, David?

Patch 1 handles locations for the C FE, this works perfectly fine.

Patch 2 handles locations for normal, non-template functions in C++
and these work fine, too.

Patch 3, the actual Fortran bits work fine and are sound (good job, Aldy
and Andrew!)

Patch 4 is a way to print the actual range to some diagnostics.
I wrote this about a year ago when only irange was available.
And from what i've heared, it's doubtful if such an as_string() is
considered useful. So i post it just for reference and don't ask for
inclusion of such a facility. Nevertheless i think that would be useful
if not just for debugging and dumping (but please to a buffer, too, so
one can hijack it ;)

Patch5 is not to be merged for obvious reasons. It is way too chatty,
doesn't run in IPA so probably ruins any "comparer" like function.
I've compiled one or two userspace, integer programs and it is not
completely off, from the looks.


C++ template functions.
=======================
I couldn't make this work, the mechanics in start_preparsed_function are
beyond what i could grok in a couple of evenings TBH.

Can you maybe help, Json or Nathan?

I tried several spots.
Directly in start_preparsed_function, in grokmethod, grokfndecl after
type = build_cp_fntype_variant, in grokfndecl if (funcdef_flag), in
finish_function but to no avail.
To me it seems that most locations are unset/ broken in the C++ FE for
the template path, or, more likely, i'm unable to operate them properly
to be fair.
I even tried the enterprise-level idea to get something vaguely around
the result decl in a template by (please don't cite me):

@@ -18214,6 +18260,17 @@ grokmethod (cp_decl_specifier_seq *declspecs,
       DECL_NO_INLINE_WARNING_P (fndecl) = 1;
     }
 
+  /* Set the location of the result decl, approximately.  */
+  tree result;
+  if ((result = DECL_RESULT (fndecl))
+      && DECL_SOURCE_LOCATION (result) == UNKNOWN_LOCATION)
+    for (int i = (int)ds_first; i != (int)ds_last; ++i)
+      if (location_t loc = declspecs->locations[i])
+       {
+         DECL_SOURCE_LOCATION (result) = loc;
+         break;
+       }
+

but there's nothing much there in my POV?

My C++ template based testcase was this:

$ cat return-narrow-2.cc ; echo EOF
namespace std { template < typename, typename > struct pair; }
template < typename > struct __mini_vector
{
  int _M_finish;
  unsigned long
  _M_space_left()
  { return _M_finish != 0; }
};
 template class __mini_vector< std::pair< long, long > >;
 template class __mini_vector< int >;
EOF

Where the locations are all confused (maybe a tad different on trunk):
$ ../gcc/xg++ -B../gcc -c -o /tmp/foo.o return-narrow-2.cc -O -Wtype-demotion
return-narrow-2.cc: In member function ‘long unsigned int __mini_vector< <template-parameter-1-1> >::_M_space_left() [with <template-parameter-1-1> = std::pair<long int, long int>]’:
return-narrow-2.cc:6:3: warning: Function ‘_M_space_left’ could return ‘bool’ [-Wtype-demotion]
    6 |   _M_space_left()
      |   ^~~~~~~~~~~~~
      |   bool
return-narrow-2.cc:6:3: note: with a range of [0,1]
return-narrow-2.cc: In member function ‘long unsigned int __mini_vector< <template-parameter-1-1> >::_M_space_left() [with <template-parameter-1-1> = int]’:
return-narrow-2.cc:6:3: warning: Function ‘_M_space_left’ could return ‘bool’ [-Wtype-demotion]
    6 |   _M_space_left()
      |   ^~~~~~~~~~~~~
      |   bool
return-narrow-2.cc:6:3: note: with a range of [0,1]


The normal C function and C++ function (non-template) dummy tests are all
fine and work as i had expected:
$ cat return-narrow.cc ; echo EOF
int xyz (int param1, int param2, int param3)
{
	if (param1 == 42)
		return 1;
	if (param2 == 17)
		return 1;
	if (param3 == 99)
		return 1;
	return 0;
}
int abc (int param1, int param2, int param3)
{
	if (param1 == 42)
		return 0;
	if (param2 == 17)
		return 0;
	if (param3 == 99)
		return 0;
	return 0;
}
const void *const * pointer_thingie (void *i)
{
	return (const void *const *)((long)i & 1);
}
int comparer (int param1, int param2, int param3) /* signed char */
{
	if (param1 >= 42)
		return 1;
	if (param2 == 17)
		return 1;
	if (param3 <= 99)
		return -1;
	return 0;
}
int
zero_to_two (int i) /* suggest: unsigned char */
{
  if (i < 0)
    return 0;
  if (i > 0)
    return 2;
  return 0;
}
int main (void) // dg-bogus "Function .main. could return .bool."
{
	return 0;
}


_Pragma("GCC visibility push(hidden)")
int my_main (int argc, char**argv) __attribute__(( visibility("default") ));
int my_main (int, char**) { return 0; }
int hidden_main (int, char**) { return 1; }
_Pragma("GCC visibility pop")


int i64c(int i)
{ /* returns [46,122] */
	i &= 0x3f;
	if (i == 0)
		return '.';
	if (i == 1)
		return '/';
	if (i < 12)
		return ('0' - 2 + i);
	if (i < 38)
		return ('A' - 12 + i);
	return ('a' - 38 + i);
}


EOF

Which gives for the normal C-flavour hunks:

$ ../gcc/xg++ -B../gcc -c -o /tmp/foo.o return-narrow.cc -O1 -Wall
return-narrow.cc: In function ‘int xyz(int, int, int)’:
return-narrow.cc:1:1: warning: Function ‘xyz’ could return ‘bool’ [-Wtype-demotion]
    1 | int xyz (int param1, int param2, int param3)
      | ^~~
      | bool
return-narrow.cc:1:1: note: with a range of [0,1]
return-narrow.cc: In function ‘int abc(int, int, int)’:
return-narrow.cc:11:1: warning: Function ‘abc’ could return ‘bool’ [-Wtype-demotion]
   11 | int abc (int param1, int param2, int param3)
      | ^~~
      | bool
return-narrow.cc:11:1: note: with a range of [0,0]
return-narrow.cc: In function ‘const void* const* pointer_thingie(void*)’:
return-narrow.cc:21:7: warning: Function ‘pointer_thingie’ could return ‘bool’ [-Wtype-demotion]
   21 | const void *const * pointer_thingie (void *i)
      |       ^~~~
      |       bool
return-narrow.cc:21:7: note: with a range of [0B,1B]
return-narrow.cc: In function ‘int comparer(int, int, int)’:
return-narrow.cc:25:1: warning: Function ‘comparer’ could return ‘signed char’ [-Wtype-demotion]
   25 | int comparer (int param1, int param2, int param3) /* signed char */
      | ^~~
      | signed char
return-narrow.cc:25:1: note: with a range of [-1,1]
return-narrow.cc: In function ‘int zero_to_two(int)’:
return-narrow.cc:35:1: warning: Function ‘zero_to_two’ could return ‘unsigned char’ [-Wtype-demotion]
   35 | int
      | ^~~
      | unsigned char
return-narrow.cc:35:1: note: with a range of [0,2]
return-narrow.cc: In function ‘int my_main(int, char**)’:
return-narrow.cc:51:1: warning: Function ‘my_main’ could return ‘bool’ [-Wtype-demotion]
   51 | int my_main (int, char**) { return 0; }
      | ^~~
      | bool
return-narrow.cc:51:1: note: with a range of [0,0]
return-narrow.cc: In function ‘int hidden_main(int, char**)’:
return-narrow.cc:52:1: warning: Function ‘hidden_main’ could return ‘bool’ [-Wtype-demotion]
   52 | int hidden_main (int, char**) { return 1; }
      | ^~~
      | bool
return-narrow.cc:52:1: note: with a range of [1,1]
return-narrow.cc: In function ‘int i64c(int)’:
return-narrow.cc:55:1: warning: Function ‘i64c’ could return ‘unsigned char’ [-Wtype-demotion]
   55 | int i64c(int i)
      | ^~~
      | unsigned char
return-narrow.cc:55:1: note: with a range of [46,122]

So, any help wrt the template case? -- many TIA!


Bernhard Reutner-Fischer (5):
  c: Set the locus of the function result decl
  c++: Set the locus of the function result decl
  Fortran: Narrow return types [PR78798]
  value-range: Add as_string diagnostics helper
  gimple: Add pass to note possible type demotions; IPA pro/demotion

 gcc/Makefile.in              |   1 +
 gcc/c-family/c.opt           |   6 +
 gcc/c/c-decl.cc              |   6 +-
 gcc/cp/decl.cc               |  15 +-
 gcc/fortran/arith.cc         |   4 +-
 gcc/fortran/arith.h          |   4 +-
 gcc/fortran/array.cc         |   8 +-
 gcc/fortran/check.cc         |   2 +-
 gcc/fortran/cpp.cc           |   3 +-
 gcc/fortran/cpp.h            |   2 +-
 gcc/fortran/dependency.cc    |   8 +-
 gcc/fortran/dependency.h     |   6 +-
 gcc/fortran/expr.cc          |   6 +-
 gcc/fortran/gfortran.h       |  51 ++--
 gcc/fortran/intrinsic.cc     |   6 +-
 gcc/fortran/io.cc            |  13 +-
 gcc/fortran/misc.cc          |   2 +-
 gcc/fortran/parse.cc         |   2 +-
 gcc/fortran/parse.h          |   2 +-
 gcc/fortran/primary.cc       |   4 +-
 gcc/fortran/resolve.cc       |  22 +-
 gcc/fortran/scanner.cc       |  20 +-
 gcc/fortran/symbol.cc        |   2 +-
 gcc/fortran/target-memory.cc |   6 +-
 gcc/fortran/trans-array.cc   |   2 +-
 gcc/fortran/trans-decl.cc    |   2 +-
 gcc/fortran/trans-types.cc   |   6 +-
 gcc/fortran/trans-types.h    |   6 +-
 gcc/fortran/trans.h          |   2 +-
 gcc/gimple-warn-types.cc     | 441 +++++++++++++++++++++++++++++++++++
 gcc/passes.def               |   1 +
 gcc/tree-pass.h              |   1 +
 gcc/value-range.cc           |  56 +++++
 gcc/value-range.h            |   3 +
 34 files changed, 618 insertions(+), 103 deletions(-)
 create mode 100644 gcc/gimple-warn-types.cc