* [PATCH 0/5] function result decl location; type demotion
@ 2022-11-12 23:45 Bernhard Reutner-Fischer
2022-11-12 23:45 ` [PATCH 1/5] c: Set the locus of the function result decl Bernhard Reutner-Fischer
` (4 more replies)
0 siblings, 5 replies; 34+ messages in thread
From: Bernhard Reutner-Fischer @ 2022-11-12 23:45 UTC (permalink / raw)
To: gcc-patches
Cc: Bernhard Reutner-Fischer, Bernhard Reutner-Fischer, fortran,
David Malcolm, Aldy Hernandez, Jason Merrill
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
--
2.38.1
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH 1/5] c: Set the locus of the function result decl
2022-11-12 23:45 [PATCH 0/5] function result decl location; type demotion Bernhard Reutner-Fischer
@ 2022-11-12 23:45 ` Bernhard Reutner-Fischer
2022-11-14 21:25 ` Joseph Myers
2022-11-12 23:45 ` [PATCH 2/5] c++: " Bernhard Reutner-Fischer
` (3 subsequent siblings)
4 siblings, 1 reply; 34+ messages in thread
From: Bernhard Reutner-Fischer @ 2022-11-12 23:45 UTC (permalink / raw)
To: gcc-patches
Cc: Bernhard Reutner-Fischer, Bernhard Reutner-Fischer, fortran,
Joseph Myers
Bootstrapped and regtested on x86_86-unknown-linux with no regressions.
Ok for trunk?
Cc: Joseph Myers <joseph@codesourcery.com>
---
gcc/c/ChangeLog:
* c-decl.cc (start_function): Set the result decl source
location to the location of the typespec.
---
gcc/c/c-decl.cc | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc
index a99b7456055..5250cb96c41 100644
--- a/gcc/c/c-decl.cc
+++ b/gcc/c/c-decl.cc
@@ -9980,6 +9980,7 @@ start_function (struct c_declspecs *declspecs, struct c_declarator *declarator,
tree decl1, old_decl;
tree restype, resdecl;
location_t loc;
+ location_t result_loc;
current_function_returns_value = 0; /* Assume, until we see it does. */
current_function_returns_null = 0;
@@ -10206,8 +10207,11 @@ start_function (struct c_declspecs *declspecs, struct c_declarator *declarator,
push_scope ();
declare_parm_level ();
+ /* Set the result decl source location to the location of the typespec. */
+ result_loc = (declspecs->locations[cdw_typespec] == UNKNOWN_LOCATION
+ ? loc : declspecs->locations[cdw_typespec]);
restype = TREE_TYPE (TREE_TYPE (current_function_decl));
- resdecl = build_decl (loc, RESULT_DECL, NULL_TREE, restype);
+ resdecl = build_decl (result_loc, RESULT_DECL, NULL_TREE, restype);
DECL_ARTIFICIAL (resdecl) = 1;
DECL_IGNORED_P (resdecl) = 1;
DECL_RESULT (current_function_decl) = resdecl;
--
2.38.1
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH 2/5] c++: Set the locus of the function result decl
2022-11-12 23:45 [PATCH 0/5] function result decl location; type demotion Bernhard Reutner-Fischer
2022-11-12 23:45 ` [PATCH 1/5] c: Set the locus of the function result decl Bernhard Reutner-Fischer
@ 2022-11-12 23:45 ` Bernhard Reutner-Fischer
2022-11-15 23:52 ` Jason Merrill
2022-11-12 23:45 ` [PATCH 3/5] Fortran: Narrow return types [PR78798] Bernhard Reutner-Fischer
` (2 subsequent siblings)
4 siblings, 1 reply; 34+ messages in thread
From: Bernhard Reutner-Fischer @ 2022-11-12 23:45 UTC (permalink / raw)
To: gcc-patches
Cc: Bernhard Reutner-Fischer, Bernhard Reutner-Fischer, fortran,
Nathan Sidwell, Jason Merrill
gcc/cp/ChangeLog:
* decl.cc (start_function): Set the result decl source location to
the location of the typespec.
---
Bootstrapped and regtested on x86_86-unknown-linux with no regressions.
Ok for trunk?
Cc: Nathan Sidwell <nathan@acm.org>
Cc: Jason Merrill <jason@redhat.com>
---
gcc/cp/decl.cc | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
index 6e98ea35a39..ed40815e645 100644
--- a/gcc/cp/decl.cc
+++ b/gcc/cp/decl.cc
@@ -17449,6 +17449,8 @@ start_function (cp_decl_specifier_seq *declspecs,
tree attrs)
{
tree decl1;
+ tree result;
+ bool ret;
decl1 = grokdeclarator (declarator, declspecs, FUNCDEF, 1, &attrs);
invoke_plugin_callbacks (PLUGIN_START_PARSE_FUNCTION, decl1);
@@ -17461,7 +17463,18 @@ 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);
+ ret = start_preparsed_function (decl1, attrs, /*flags=*/SF_DEFAULT);
+
+ /* decl1 might be ggc_freed here. */
+ decl1 = current_function_decl;
+
+ /* 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
--
2.38.1
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH 3/5] Fortran: Narrow return types [PR78798]
2022-11-12 23:45 [PATCH 0/5] function result decl location; type demotion Bernhard Reutner-Fischer
2022-11-12 23:45 ` [PATCH 1/5] c: Set the locus of the function result decl Bernhard Reutner-Fischer
2022-11-12 23:45 ` [PATCH 2/5] c++: " Bernhard Reutner-Fischer
@ 2022-11-12 23:45 ` Bernhard Reutner-Fischer
2022-11-13 10:13 ` Janne Blomqvist
2022-11-12 23:45 ` [PATCH 4/5] value-range: Add as_string diagnostics helper Bernhard Reutner-Fischer
2022-11-12 23:45 ` [PATCH 5/5] gimple: Add pass to note possible type demotions; IPA pro/demotion; DO NOT MERGE Bernhard Reutner-Fischer
4 siblings, 1 reply; 34+ messages in thread
From: Bernhard Reutner-Fischer @ 2022-11-12 23:45 UTC (permalink / raw)
To: gcc-patches; +Cc: Bernhard Reutner-Fischer, Bernhard Reutner-Fischer, fortran
gcc/fortran/ChangeLog:
* arith.cc (compare_complex): Use narrower return type.
(gfc_compare_string): Likewise.
* arith.h (gfc_compare_string): Same.
(gfc_compare_with_Cstring): Ditto.
* array.cc (compare_bounds): Ditto.
(gfc_compare_array_spec): Likewise.
(is_constant_element): Likewise.
(gfc_constant_ac): Likewise.
* check.cc (dim_rank_check): Likewise.
* cpp.cc (gfc_cpp_init_options): Likewise.
(dump_macro): Likewise.
* cpp.h (gfc_cpp_handle_option): Likewise.
* dependency.cc (gfc_ref_needs_temporary_p): Likewise.
(gfc_check_argument_dependency): Likewise.
(gfc_check_fncall_dependency): Likewise.
(ref_same_as_full_array): Likewise.
* dependency.h (gfc_check_fncall_dependency): Likewise.
(gfc_dep_resolver): Likewise.
(gfc_are_equivalenced_arrays): Likewise.
* expr.cc (gfc_copy_ref): Likewise.
(gfc_kind_max): Likewise.
(numeric_type): Likewise.
* gfortran.h (gfc_at_end): Likewise.
(gfc_at_eof): Likewise.
(gfc_at_bol): Likewise.
(gfc_at_eol): Likewise.
(gfc_check_include): Likewise.
(gfc_define_undef_line): Likewise.
(gfc_wide_is_printable): Likewise.
(gfc_wide_is_digit): Likewise.
(gfc_wide_fits_in_byte): Likewise.
(get_c_kind): Likewise.
(gfc_find_sym_tree): Likewise.
(gfc_generic_intrinsic): Likewise.
(gfc_specific_intrinsic): Likewise.
(gfc_intrinsic_actual_ok): Likewise.
(gfc_has_vector_index): Likewise.
(gfc_numeric_ts): Likewise.
(gfc_impure_variable): Likewise.
(gfc_pure): Likewise.
(gfc_implicit_pure): Likewise.
(gfc_elemental): Likewise.
(gfc_pure_function): Likewise.
(gfc_implicit_pure_function): Likewise.
(gfc_compare_array_spec): Likewise.
(gfc_constant_ac): Likewise.
(gfc_expanded_ac): Likewise.
(gfc_check_digit): Likewise.
* intrinsic.cc (gfc_find_subroutine): Likewise.
(gfc_generic_intrinsic): Likewise.
(gfc_specific_intrinsic): Likewise.
* io.cc (compare_to_allowed_values): Likewise.
* misc.cc (gfc_done_2): Likewise.
* parse.cc: Likewise.
* parse.h (gfc_check_do_variable): Likewise.
* primary.cc (gfc_check_digit): Likewise.
* resolve.cc (resolve_structure_cons): Likewise.
(pure_stmt_function): Likewise.
(gfc_pure_function): Likewise.
(impure_stmt_fcn): Likewise.
(resolve_forall_iterators): Likewise.
(resolve_data): Likewise.
(gfc_impure_variable): Likewise.
(gfc_pure): Likewise.
(gfc_unset_implicit_pure): Likewise.
* scanner.cc (wide_is_ascii): Likewise.
(gfc_wide_toupper): Likewise.
(gfc_open_included_file): Likewise.
(gfc_at_end): Likewise.
(gfc_at_eof): Likewise.
(gfc_at_bol): Likewise.
(skip_comment_line): Likewise.
(gfc_gobble_whitespace): Likewise.
* symbol.cc (gfc_find_symtree_in_proc): Likewise.
* target-memory.cc (size_integer): Likewise.
(size_complex): Likewise.
* trans-array.cc: Likewise.
* trans-decl.cc (gfc_set_decl_assembler_name): Likewise.
* trans-types.cc (gfc_get_element_type): Likewise.
(gfc_add_field_to_struct): Likewise.
* trans-types.h (gfc_copy_dt_decls_ifequal): Likewise.
(gfc_return_by_reference): Likewise.
(gfc_is_nodesc_array): Likewise.
* trans.h (gfc_can_put_var_on_stack): Likewise.
---
Bootstrapped and regtested on x86_86-unknown-linux with no regressions.
Ok for trunk?
Cc: fortran@gcc.gnu.org
---
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 +-
25 files changed, 90 insertions(+), 101 deletions(-)
diff --git a/gcc/fortran/arith.cc b/gcc/fortran/arith.cc
index fc9224ebc5c..55f35ea66be 100644
--- a/gcc/fortran/arith.cc
+++ b/gcc/fortran/arith.cc
@@ -1135,7 +1135,7 @@ compare_complex (gfc_expr *op1, gfc_expr *op2)
strings. We return -1 for a < b, 0 for a == b and 1 for a > b.
We use the processor's default collating sequence. */
-int
+signed char
gfc_compare_string (gfc_expr *a, gfc_expr *b)
{
size_t len, alen, blen, i;
@@ -1162,7 +1162,7 @@ gfc_compare_string (gfc_expr *a, gfc_expr *b)
}
-int
+signed char
gfc_compare_with_Cstring (gfc_expr *a, const char *b, bool case_sensitive)
{
size_t len, alen, blen, i;
diff --git a/gcc/fortran/arith.h b/gcc/fortran/arith.h
index 4a79c985231..8aeb7242166 100644
--- a/gcc/fortran/arith.h
+++ b/gcc/fortran/arith.h
@@ -33,8 +33,8 @@ void gfc_set_model (mpfr_t);
arith gfc_range_check (gfc_expr *);
int gfc_compare_expr (gfc_expr *, gfc_expr *, gfc_intrinsic_op);
-int gfc_compare_string (gfc_expr *, gfc_expr *);
-int gfc_compare_with_Cstring (gfc_expr *, const char *, bool);
+signed char gfc_compare_string (gfc_expr *, gfc_expr *);
+signed char gfc_compare_with_Cstring (gfc_expr *, const char *, bool);
/* Constant folding for gfc_expr trees. */
diff --git a/gcc/fortran/array.cc b/gcc/fortran/array.cc
index bbdb5b392fc..281f95a0a7e 100644
--- a/gcc/fortran/array.cc
+++ b/gcc/fortran/array.cc
@@ -985,7 +985,7 @@ compare_bounds (gfc_expr *bound1, gfc_expr *bound2)
/* Compares two array specifications. They must be constant or deferred
shape. */
-int
+bool
gfc_compare_array_spec (gfc_array_spec *as1, gfc_array_spec *as2)
{
int i;
@@ -1030,7 +1030,7 @@ gfc_compare_array_spec (gfc_array_spec *as1, gfc_array_spec *as2)
use the symbol as an implied-DO iterator. Returns nonzero if a
duplicate was found. */
-static int
+static bool
check_duplicate_iterator (gfc_constructor_base base, gfc_symbol *master)
{
gfc_constructor *c;
@@ -1973,7 +1973,7 @@ is_constant_element (gfc_expr *e)
i=1,100000000) /) will take a while as* opposed to a more clever
function that traverses the expression tree. FIXME. */
-int
+bool
gfc_constant_ac (gfc_expr *e)
{
expand_info expand_save;
@@ -1996,7 +1996,7 @@ gfc_constant_ac (gfc_expr *e)
/* Returns nonzero if an array constructor has been completely
expanded (no iterators) and zero if iterators are present. */
-int
+bool
gfc_expanded_ac (gfc_expr *e)
{
gfc_constructor *c;
diff --git a/gcc/fortran/check.cc b/gcc/fortran/check.cc
index 91d87a1b2c1..e0ca1e8d7ca 100644
--- a/gcc/fortran/check.cc
+++ b/gcc/fortran/check.cc
@@ -1156,7 +1156,7 @@ dim_rank_check (gfc_expr *dim, gfc_expr *array, int allow_assumed)
dimension bi, returning 0 if they are known not to be identical,
and 1 if they are identical, or if this cannot be determined. */
-static int
+static bool
identical_dimen_shape (gfc_expr *a, int ai, gfc_expr *b, int bi)
{
mpz_t a_size, b_size;
diff --git a/gcc/fortran/cpp.cc b/gcc/fortran/cpp.cc
index 364bd0d2a85..9ae8046c4bc 100644
--- a/gcc/fortran/cpp.cc
+++ b/gcc/fortran/cpp.cc
@@ -297,7 +297,7 @@ gfc_cpp_init_options (unsigned int decoded_options_count,
gfc_cpp_option.deferred_opt_count = 0;
}
-int
+bool
gfc_cpp_handle_option (size_t scode, const char *arg, int value ATTRIBUTE_UNUSED)
{
int result = 1;
@@ -749,7 +749,6 @@ gfc_cpp_add_include_path_after (char *path, bool user_supplied)
static void scan_translation_unit_trad (cpp_reader *);
static void account_for_newlines (const unsigned char *, size_t);
-static int dump_macro (cpp_reader *, cpp_hashnode *, void *);
static void print_line (location_t, const char *);
static void maybe_print_line (location_t);
diff --git a/gcc/fortran/cpp.h b/gcc/fortran/cpp.h
index 28673e32aa2..a003d5922d6 100644
--- a/gcc/fortran/cpp.h
+++ b/gcc/fortran/cpp.h
@@ -39,7 +39,7 @@ void gfc_cpp_init (void);
void gfc_cpp_init_options (unsigned int decoded_options_count,
struct cl_decoded_option *decoded_options);
-int gfc_cpp_handle_option(size_t scode, const char *arg, int value);
+bool gfc_cpp_handle_option(size_t scode, const char *arg, int value);
void gfc_cpp_post_options (bool);
diff --git a/gcc/fortran/dependency.cc b/gcc/fortran/dependency.cc
index ab3bd36f74e..dee50cc8926 100644
--- a/gcc/fortran/dependency.cc
+++ b/gcc/fortran/dependency.cc
@@ -921,7 +921,7 @@ gfc_ref_needs_temporary_p (gfc_ref *ref)
}
-static int
+static bool
gfc_is_data_pointer (gfc_expr *e)
{
gfc_ref *ref;
@@ -1091,7 +1091,7 @@ gfc_check_argument_dependency (gfc_expr *other, sym_intent intent,
/* Like gfc_check_argument_dependency, but check all the arguments in ACTUAL.
FNSYM is the function being called, or NULL if not known. */
-int
+bool
gfc_check_fncall_dependency (gfc_expr *other, sym_intent intent,
gfc_symbol *fnsym, gfc_actual_arglist *actual,
gfc_dep_check elemental)
@@ -1137,7 +1137,7 @@ gfc_check_fncall_dependency (gfc_expr *other, sym_intent intent,
e1->ref and e2->ref to determine whether the actually accessed
portions of these variables/arrays potentially overlap. */
-int
+bool
gfc_are_equivalenced_arrays (gfc_expr *e1, gfc_expr *e2)
{
gfc_equiv_list *l;
@@ -2093,7 +2093,7 @@ ref_same_as_full_array (gfc_ref *full_ref, gfc_ref *ref)
there is some kind of overlap.
0 : array references are identical or not overlapping. */
-int
+bool
gfc_dep_resolver (gfc_ref *lref, gfc_ref *rref, gfc_reverse *reverse,
bool identical)
{
diff --git a/gcc/fortran/dependency.h b/gcc/fortran/dependency.h
index 339be76a8d0..81114054eb8 100644
--- a/gcc/fortran/dependency.h
+++ b/gcc/fortran/dependency.h
@@ -32,13 +32,13 @@ enum gfc_dep_check
bool gfc_ref_needs_temporary_p (gfc_ref *);
bool gfc_full_array_ref_p (gfc_ref *, bool *);
gfc_expr *gfc_get_noncopying_intrinsic_argument (gfc_expr *);
-int gfc_check_fncall_dependency (gfc_expr *, sym_intent, gfc_symbol *,
+bool gfc_check_fncall_dependency (gfc_expr *, sym_intent, gfc_symbol *,
gfc_actual_arglist *, gfc_dep_check);
int gfc_check_dependency (gfc_expr *, gfc_expr *, bool);
int gfc_expr_is_one (gfc_expr *, int);
-int gfc_dep_resolver (gfc_ref *, gfc_ref *, gfc_reverse *,
+bool gfc_dep_resolver (gfc_ref *, gfc_ref *, gfc_reverse *,
bool identical = false);
-int gfc_are_equivalenced_arrays (gfc_expr *, gfc_expr *);
+bool gfc_are_equivalenced_arrays (gfc_expr *, gfc_expr *);
gfc_expr * gfc_discard_nops (gfc_expr *);
diff --git a/gcc/fortran/expr.cc b/gcc/fortran/expr.cc
index 69d0b57c688..e095e74d290 100644
--- a/gcc/fortran/expr.cc
+++ b/gcc/fortran/expr.cc
@@ -787,7 +787,7 @@ gfc_copy_ref (gfc_ref *src)
/* Detect whether an expression has any vector index array references. */
-int
+bool
gfc_has_vector_index (gfc_expr *e)
{
gfc_ref *ref;
@@ -877,7 +877,7 @@ gfc_kind_max (gfc_expr *e1, gfc_expr *e2)
/* Returns nonzero if the type is numeric, zero otherwise. */
-static int
+static bool
numeric_type (bt type)
{
return type == BT_COMPLEX || type == BT_REAL || type == BT_INTEGER;
@@ -886,7 +886,7 @@ numeric_type (bt type)
/* Returns nonzero if the typespec is a numeric type, zero otherwise. */
-int
+bool
gfc_numeric_ts (gfc_typespec *ts)
{
return numeric_type (ts->type);
diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h
index 6bd8800ecf8..4cc969d7dc8 100644
--- a/gcc/fortran/gfortran.h
+++ b/gcc/fortran/gfortran.h
@@ -3202,17 +3202,16 @@ void gfc_release_include_path (void);
void gfc_check_include_dirs (bool);
FILE *gfc_open_included_file (const char *, bool, bool);
-int gfc_at_end (void);
-int gfc_at_eof (void);
-int gfc_at_bol (void);
-int gfc_at_eol (void);
+bool gfc_at_end (void);
+bool gfc_at_eof (void);
+bool gfc_at_bol (void);
+bool gfc_at_eol (void);
void gfc_advance_line (void);
-int gfc_check_include (void);
-int gfc_define_undef_line (void);
+bool gfc_define_undef_line (void);
-int gfc_wide_is_printable (gfc_char_t);
-int gfc_wide_is_digit (gfc_char_t);
-int gfc_wide_fits_in_byte (gfc_char_t);
+bool gfc_wide_is_printable (gfc_char_t);
+bool gfc_wide_is_digit (gfc_char_t);
+bool gfc_wide_fits_in_byte (gfc_char_t);
gfc_char_t gfc_wide_tolower (gfc_char_t);
gfc_char_t gfc_wide_toupper (gfc_char_t);
size_t gfc_wide_strlen (const gfc_char_t *);
@@ -3258,7 +3257,7 @@ void gfc_init_2 (void);
void gfc_done_1 (void);
void gfc_done_2 (void);
-int get_c_kind (const char *, CInteropKind_t *);
+signed char get_c_kind (const char *, CInteropKind_t *);
const char *gfc_closest_fuzzy_match (const char *, char **);
static inline void
@@ -3469,7 +3468,7 @@ void gfc_release_symbol (gfc_symbol *&);
gfc_symbol *gfc_new_symbol (const char *, gfc_namespace *);
gfc_symtree* gfc_find_symtree_in_proc (const char *, gfc_namespace *);
int gfc_find_symbol (const char *, gfc_namespace *, int, gfc_symbol **);
-int gfc_find_sym_tree (const char *, gfc_namespace *, int, gfc_symtree **);
+bool gfc_find_sym_tree (const char *, gfc_namespace *, int, gfc_symtree **);
int gfc_get_symbol (const char *, gfc_namespace *, gfc_symbol **);
bool gfc_verify_c_interop (gfc_typespec *);
bool gfc_verify_c_interop_param (gfc_symbol *);
@@ -3549,10 +3548,10 @@ bool gfc_convert_type (gfc_expr *, gfc_typespec *, int);
bool gfc_convert_type_warn (gfc_expr *, gfc_typespec *, int, int,
bool array = false);
bool gfc_convert_chartype (gfc_expr *, gfc_typespec *);
-int gfc_generic_intrinsic (const char *);
-int gfc_specific_intrinsic (const char *);
+bool gfc_generic_intrinsic (const char *);
+bool gfc_specific_intrinsic (const char *);
bool gfc_is_intrinsic (gfc_symbol*, int, locus);
-int gfc_intrinsic_actual_ok (const char *, const bool);
+bool gfc_intrinsic_actual_ok (const char *, const bool);
gfc_intrinsic_sym *gfc_find_function (const char *);
gfc_intrinsic_sym *gfc_find_subroutine (const char *);
gfc_intrinsic_sym *gfc_intrinsic_function_by_id (gfc_isym_id);
@@ -3630,7 +3629,7 @@ void gfc_type_convert_binary (gfc_expr *, int);
bool gfc_is_constant_expr (gfc_expr *);
bool gfc_simplify_expr (gfc_expr *, int);
bool gfc_try_simplify_expr (gfc_expr *, int);
-int gfc_has_vector_index (gfc_expr *);
+bool gfc_has_vector_index (gfc_expr *);
gfc_expr *gfc_get_expr (void);
gfc_expr *gfc_get_array_expr (bt type, int kind, locus *);
@@ -3654,7 +3653,7 @@ gfc_ref* gfc_copy_ref (gfc_ref*);
bool gfc_specification_expr (gfc_expr *);
-int gfc_numeric_ts (gfc_typespec *);
+bool gfc_numeric_ts (gfc_typespec *);
int gfc_kind_max (gfc_expr *, gfc_expr *);
bool gfc_check_conformance (gfc_expr *, gfc_expr *, const char *, ...) ATTRIBUTE_PRINTF_3;
@@ -3718,11 +3717,11 @@ void gfc_resolve (gfc_namespace *);
void gfc_resolve_code (gfc_code *, gfc_namespace *);
void gfc_resolve_blocks (gfc_code *, gfc_namespace *);
void gfc_resolve_formal_arglist (gfc_symbol *);
-int gfc_impure_variable (gfc_symbol *);
-int gfc_pure (gfc_symbol *);
-int gfc_implicit_pure (gfc_symbol *);
+bool gfc_impure_variable (gfc_symbol *);
+bool gfc_pure (gfc_symbol *);
+bool gfc_implicit_pure (gfc_symbol *);
void gfc_unset_implicit_pure (gfc_symbol *);
-int gfc_elemental (gfc_symbol *);
+bool gfc_elemental (gfc_symbol *);
bool gfc_resolve_iterator (gfc_iterator *, bool, bool);
bool find_forall_index (gfc_expr *, gfc_symbol *, int);
bool gfc_resolve_index (gfc_expr *, int);
@@ -3736,8 +3735,8 @@ bool gfc_resolve_intrinsic (gfc_symbol *, locus *);
bool gfc_explicit_interface_required (gfc_symbol *, char *, int);
extern int gfc_do_concurrent_flag;
const char* gfc_lookup_function_fuzzy (const char *, gfc_symtree *);
-int gfc_pure_function (gfc_expr *e, const char **name);
-int gfc_implicit_pure_function (gfc_expr *e);
+bool gfc_pure_function (gfc_expr *e, const char **name);
+bool gfc_implicit_pure_function (gfc_expr *e);
/* array.cc */
@@ -3750,12 +3749,12 @@ bool gfc_set_array_spec (gfc_symbol *, gfc_array_spec *, locus *);
gfc_array_spec *gfc_copy_array_spec (gfc_array_spec *);
bool gfc_resolve_array_spec (gfc_array_spec *, int);
-int gfc_compare_array_spec (gfc_array_spec *, gfc_array_spec *);
+bool gfc_compare_array_spec (gfc_array_spec *, gfc_array_spec *);
void gfc_simplify_iterator_var (gfc_expr *);
bool gfc_expand_constructor (gfc_expr *, bool);
-int gfc_constant_ac (gfc_expr *);
-int gfc_expanded_ac (gfc_expr *);
+bool gfc_constant_ac (gfc_expr *);
+bool gfc_expanded_ac (gfc_expr *);
bool gfc_resolve_character_array_constructor (gfc_expr *);
bool gfc_resolve_array_constructor (gfc_expr *);
bool gfc_check_constructor_type (gfc_expr *);
@@ -3840,7 +3839,7 @@ symbol_attribute gfc_expr_attr (gfc_expr *);
symbol_attribute gfc_caf_attr (gfc_expr *, bool i = false, bool *r = NULL);
match gfc_match_rvalue (gfc_expr **);
match gfc_match_varspec (gfc_expr*, int, bool, bool);
-int gfc_check_digit (char, int);
+bool gfc_check_digit (char, int);
bool gfc_is_function_return_value (gfc_symbol *, gfc_namespace *);
bool gfc_convert_to_structure_constructor (gfc_expr *, gfc_symbol *,
gfc_expr **,
diff --git a/gcc/fortran/intrinsic.cc b/gcc/fortran/intrinsic.cc
index e89131f5a71..1d3702ff313 100644
--- a/gcc/fortran/intrinsic.cc
+++ b/gcc/fortran/intrinsic.cc
@@ -1106,7 +1106,7 @@ gfc_find_subroutine (const char *name)
/* Given a string, figure out if it is the name of a generic intrinsic
function or not. */
-int
+bool
gfc_generic_intrinsic (const char *name)
{
gfc_intrinsic_sym *sym;
@@ -1119,7 +1119,7 @@ gfc_generic_intrinsic (const char *name)
/* Given a string, figure out if it is the name of a specific
intrinsic function or not. */
-int
+bool
gfc_specific_intrinsic (const char *name)
{
gfc_intrinsic_sym *sym;
@@ -1131,7 +1131,7 @@ gfc_specific_intrinsic (const char *name)
/* Given a string, figure out if it is the name of an intrinsic function
or subroutine allowed as an actual argument or not. */
-int
+bool
gfc_intrinsic_actual_ok (const char *name, const bool subroutine_flag)
{
gfc_intrinsic_sym *sym;
diff --git a/gcc/fortran/io.cc b/gcc/fortran/io.cc
index 902aa19f55a..488264de4b4 100644
--- a/gcc/fortran/io.cc
+++ b/gcc/fortran/io.cc
@@ -2010,15 +2010,6 @@ gfc_free_open (gfc_open *open)
free (open);
}
-
-static int
-compare_to_allowed_values (const char *specifier, const char *allowed[],
- const char *allowed_f2003[],
- const char *allowed_gnu[], gfc_char_t *value,
- const char *statement, bool warn, locus *where,
- int *num = NULL);
-
-
static bool
check_open_constraints (gfc_open *open, locus *where);
@@ -2062,12 +2053,12 @@ gfc_resolve_open (gfc_open *open, locus *where)
value if it is not allowed. */
-static int
+static bool
compare_to_allowed_values (const char *specifier, const char *allowed[],
const char *allowed_f2003[],
const char *allowed_gnu[], gfc_char_t *value,
const char *statement, bool warn, locus *where,
- int *num)
+ int *num = NULL)
{
int i;
unsigned int len;
diff --git a/gcc/fortran/misc.cc b/gcc/fortran/misc.cc
index af363472641..270ea817f61 100644
--- a/gcc/fortran/misc.cc
+++ b/gcc/fortran/misc.cc
@@ -387,7 +387,7 @@ gfc_done_2 (void)
/* Returns the index into the table of C interoperable kinds where the
kind with the given name (c_kind_name) was found. */
-int
+signed char
get_c_kind(const char *c_kind_name, CInteropKind_t kinds_table[])
{
int index = 0;
diff --git a/gcc/fortran/parse.cc b/gcc/fortran/parse.cc
index f04fd13cc69..352191e650e 100644
--- a/gcc/fortran/parse.cc
+++ b/gcc/fortran/parse.cc
@@ -4703,7 +4703,7 @@ done:
context that causes it to become redefined. If the symbol is an
iterator, we generate an error message and return nonzero. */
-int
+bool
gfc_check_do_variable (gfc_symtree *st)
{
gfc_state_data *s;
diff --git a/gcc/fortran/parse.h b/gcc/fortran/parse.h
index 013aeaedc03..44b3a5079c0 100644
--- a/gcc/fortran/parse.h
+++ b/gcc/fortran/parse.h
@@ -63,7 +63,7 @@ extern gfc_state_data *gfc_state_stack;
#define gfc_comp_struct(s) \
((s) == COMP_DERIVED || (s) == COMP_STRUCTURE || (s) == COMP_MAP)
-int gfc_check_do_variable (gfc_symtree *);
+bool gfc_check_do_variable (gfc_symtree *);
bool gfc_find_state (gfc_compile_state);
gfc_state_data *gfc_enclosing_unit (gfc_compile_state *);
const char *gfc_ascii_statement (gfc_statement, bool strip_sentinel = false) ;
diff --git a/gcc/fortran/primary.cc b/gcc/fortran/primary.cc
index 19f2e78c8ff..52457cc8277 100644
--- a/gcc/fortran/primary.cc
+++ b/gcc/fortran/primary.cc
@@ -109,10 +109,10 @@ get_kind (int *is_iso_c)
/* Given a character and a radix, see if the character is a valid
digit in that radix. */
-int
+bool
gfc_check_digit (char c, int radix)
{
- int r;
+ bool r;
switch (radix)
{
diff --git a/gcc/fortran/resolve.cc b/gcc/fortran/resolve.cc
index 9202e2f10ad..39074273e95 100644
--- a/gcc/fortran/resolve.cc
+++ b/gcc/fortran/resolve.cc
@@ -1562,7 +1562,7 @@ resolve_structure_cons (gfc_expr *expr, int init)
/* Returns 0 if a symbol was not declared with a type or
attribute declaration statement, nonzero otherwise. */
-static int
+static bool
was_declared (gfc_symbol *sym)
{
symbol_attribute a;
@@ -3066,13 +3066,13 @@ is_external_proc (gfc_symbol *sym)
/* Figure out if a function reference is pure or not. Also set the name
of the function for a potential error message. Return nonzero if the
function is PURE, zero if not. */
-static int
+static bool
pure_stmt_function (gfc_expr *, gfc_symbol *);
-int
+bool
gfc_pure_function (gfc_expr *e, const char **name)
{
- int pure;
+ bool pure;
gfc_component *comp;
*name = NULL;
@@ -3112,7 +3112,7 @@ gfc_pure_function (gfc_expr *e, const char **name)
/* Check if the expression is a reference to an implicitly pure function. */
-int
+bool
gfc_implicit_pure_function (gfc_expr *e)
{
gfc_component *comp = gfc_get_proc_ptr_comp (e);
@@ -3143,7 +3143,7 @@ impure_stmt_fcn (gfc_expr *e, gfc_symbol *sym,
}
-static int
+static bool
pure_stmt_function (gfc_expr *e, gfc_symbol *sym)
{
return gfc_traverse_expr (e, sym, impure_stmt_fcn, 0) ? 0 : 1;
@@ -7524,7 +7524,7 @@ resolve_forall_iterators (gfc_forall_iterator *it)
PRIVATE. The search is recursive if necessary. Returns zero if no
inaccessible components are found, nonzero otherwise. */
-static int
+static bool
derived_inaccessible (gfc_symbol *sym)
{
gfc_component *c;
@@ -16757,7 +16757,7 @@ resolve_data (gfc_data *d)
/* Determines if a variable is not 'pure', i.e., not assignable within a pure
procedure. Returns zero if assignment is OK, nonzero if there is a
problem. */
-int
+bool
gfc_impure_variable (gfc_symbol *sym)
{
gfc_symbol *proc;
@@ -16792,7 +16792,7 @@ gfc_impure_variable (gfc_symbol *sym)
/* Test whether a symbol is pure or not. For a NULL pointer, checks if the
current namespace is inside a pure procedure. */
-int
+bool
gfc_pure (gfc_symbol *sym)
{
symbol_attribute attr;
@@ -16824,7 +16824,7 @@ gfc_pure (gfc_symbol *sym)
checks if the current namespace is implicitly pure. Note that this
function returns false for a PURE procedure. */
-int
+bool
gfc_implicit_pure (gfc_symbol *sym)
{
gfc_namespace *ns;
@@ -16878,7 +16878,7 @@ gfc_unset_implicit_pure (gfc_symbol *sym)
/* Test whether the current procedure is elemental or not. */
-int
+bool
gfc_elemental (gfc_symbol *sym)
{
symbol_attribute attr;
diff --git a/gcc/fortran/scanner.cc b/gcc/fortran/scanner.cc
index fa1d9cba394..cae26a9f2bc 100644
--- a/gcc/fortran/scanner.cc
+++ b/gcc/fortran/scanner.cc
@@ -56,7 +56,7 @@ gfc_directorylist *include_dirs, *intrinsic_modules_dirs;
static gfc_file *file_head, *current_file;
-static int continue_flag, end_flag, gcc_attribute_flag;
+static bool continue_flag, end_flag, gcc_attribute_flag;
/* If !$omp/!$acc occurred in current comment line. */
static int openmp_flag, openacc_flag;
static int continue_count, continue_line;
@@ -86,7 +86,7 @@ static gfc_char_t *last_error_char;
/* Functions dealing with our wide characters (gfc_char_t) and
sequences of such characters. */
-int
+bool
gfc_wide_fits_in_byte (gfc_char_t c)
{
return (c <= UCHAR_MAX);
@@ -98,7 +98,7 @@ wide_is_ascii (gfc_char_t c)
return (gfc_wide_fits_in_byte (c) && ((unsigned char) c & ~0x7f) == 0);
}
-int
+bool
gfc_wide_is_printable (gfc_char_t c)
{
return (gfc_wide_fits_in_byte (c) && ISPRINT ((unsigned char) c));
@@ -116,7 +116,7 @@ gfc_wide_toupper (gfc_char_t c)
return (wide_is_ascii (c) ? (gfc_char_t) TOUPPER((unsigned char) c) : c);
}
-int
+bool
gfc_wide_is_digit (gfc_char_t c)
{
return (c >= '0' && c <= '9');
@@ -518,7 +518,7 @@ gfc_open_included_file (const char *name, bool include_cwd, bool module)
/* Test to see if we're at the end of the main source file. */
-int
+bool
gfc_at_end (void)
{
return end_flag;
@@ -527,7 +527,7 @@ gfc_at_end (void)
/* Test to see if we're at the end of the current file. */
-int
+bool
gfc_at_eof (void)
{
if (gfc_at_end ())
@@ -545,7 +545,7 @@ gfc_at_eof (void)
/* Test to see if we're at the beginning of a new line. */
-int
+bool
gfc_at_bol (void)
{
if (gfc_at_eof ())
@@ -557,7 +557,7 @@ gfc_at_bol (void)
/* Test to see if we're at the end of a line. */
-int
+bool
gfc_at_eol (void)
{
if (gfc_at_eof ())
@@ -702,7 +702,7 @@ skip_comment_line (void)
}
-int
+bool
gfc_define_undef_line (void)
{
char *tmp;
@@ -1803,7 +1803,7 @@ gfc_gobble_whitespace (void)
easily report line and column numbers consistent with other
parts of gfortran. */
-static int
+static bool
load_line (FILE *input, gfc_char_t **pbuf, int *pbuflen, const int *first_char)
{
int c, maxlen, i, preprocessor_flag, buflen = *pbuflen;
diff --git a/gcc/fortran/symbol.cc b/gcc/fortran/symbol.cc
index 49fb37864bd..65860938681 100644
--- a/gcc/fortran/symbol.cc
+++ b/gcc/fortran/symbol.cc
@@ -3210,7 +3210,7 @@ gfc_find_symtree_in_proc (const char* name, gfc_namespace* ns)
any parent namespaces if requested by a nonzero parent_flag.
Returns nonzero if the name is ambiguous. */
-int
+bool
gfc_find_sym_tree (const char *name, gfc_namespace *ns, int parent_flag,
gfc_symtree **result)
{
diff --git a/gcc/fortran/target-memory.cc b/gcc/fortran/target-memory.cc
index 7ce7d736629..8e32c56f479 100644
--- a/gcc/fortran/target-memory.cc
+++ b/gcc/fortran/target-memory.cc
@@ -36,14 +36,14 @@ along with GCC; see the file COPYING3. If not see
/* Calculate the size of an expression. */
-static size_t
+static short unsigned int
size_integer (int kind)
{
return GET_MODE_SIZE (SCALAR_INT_TYPE_MODE (gfc_get_int_type (kind)));
}
-static size_t
+static short unsigned int
size_float (int kind)
{
return GET_MODE_SIZE (SCALAR_FLOAT_TYPE_MODE (gfc_get_real_type (kind)));
@@ -57,7 +57,7 @@ size_complex (int kind)
}
-static size_t
+static short unsigned int
size_logical (int kind)
{
return GET_MODE_SIZE (SCALAR_INT_TYPE_MODE (gfc_get_logical_type (kind)));
diff --git a/gcc/fortran/trans-array.cc b/gcc/fortran/trans-array.cc
index 514cb057afb..dc84fbaeb20 100644
--- a/gcc/fortran/trans-array.cc
+++ b/gcc/fortran/trans-array.cc
@@ -4931,7 +4931,7 @@ done:
/* Return true if both symbols could refer to the same data object. Does
not take account of aliasing due to equivalence statements. */
-static int
+static bool
symbols_could_alias (gfc_symbol *lsym, gfc_symbol *rsym, bool lsym_pointer,
bool lsym_target, bool rsym_pointer, bool rsym_target)
{
diff --git a/gcc/fortran/trans-decl.cc b/gcc/fortran/trans-decl.cc
index 63515b9072a..ba855c4569e 100644
--- a/gcc/fortran/trans-decl.cc
+++ b/gcc/fortran/trans-decl.cc
@@ -483,7 +483,7 @@ gfc_set_decl_assembler_name (tree decl, tree name)
/* Returns true if a variable of specified size should go on the stack. */
-int
+bool
gfc_can_put_var_on_stack (tree size)
{
unsigned HOST_WIDE_INT low;
diff --git a/gcc/fortran/trans-types.cc b/gcc/fortran/trans-types.cc
index def7552ac67..99a28415cb3 100644
--- a/gcc/fortran/trans-types.cc
+++ b/gcc/fortran/trans-types.cc
@@ -1374,7 +1374,7 @@ gfc_get_element_type (tree type)
/* Returns true if the array sym does not require a descriptor. */
-int
+bool
gfc_is_nodesc_array (gfc_symbol * sym)
{
symbol_attribute *array_attr;
@@ -2451,7 +2451,7 @@ gfc_add_field_to_struct (tree context, tree name, tree type, tree **chain)
the two derived type symbols are "equal", as described
in 4.4.2 and resolved by gfc_compare_derived_types. */
-int
+bool
gfc_copy_dt_decls_ifequal (gfc_symbol *from, gfc_symbol *to,
bool from_gsym)
{
@@ -2940,7 +2940,7 @@ copy_derived_types:
}
-int
+bool
gfc_return_by_reference (gfc_symbol * sym)
{
if (!sym->attr.function)
diff --git a/gcc/fortran/trans-types.h b/gcc/fortran/trans-types.h
index 6a360de69f6..297b93702d8 100644
--- a/gcc/fortran/trans-types.h
+++ b/gcc/fortran/trans-types.h
@@ -88,7 +88,7 @@ tree gfc_get_character_type_len_for_eltype (tree, tree);
tree gfc_sym_type (gfc_symbol *, bool is_bind_c_arg = false);
tree gfc_get_cfi_type (int dimen, bool restricted);
tree gfc_typenode_for_spec (gfc_typespec *, int c = 0);
-int gfc_copy_dt_decls_ifequal (gfc_symbol *, gfc_symbol *, bool);
+bool gfc_copy_dt_decls_ifequal (gfc_symbol *, gfc_symbol *, bool);
tree gfc_get_function_type (gfc_symbol *, gfc_actual_arglist *args = NULL,
const char *fnspec = NULL);
@@ -109,10 +109,10 @@ tree gfc_add_field_to_struct (tree, tree, tree, tree **);
void gfc_finish_type (tree);
/* Some functions have an extra parameter for the return value. */
-int gfc_return_by_reference (gfc_symbol *);
+bool gfc_return_by_reference (gfc_symbol *);
/* Returns true if the array sym does not require a descriptor. */
-int gfc_is_nodesc_array (gfc_symbol *);
+bool gfc_is_nodesc_array (gfc_symbol *);
/* Return the DTYPE for an array. */
tree gfc_get_dtype_rank_type (int, tree);
diff --git a/gcc/fortran/trans.h b/gcc/fortran/trans.h
index bc9035c1717..1ea30156675 100644
--- a/gcc/fortran/trans.h
+++ b/gcc/fortran/trans.h
@@ -664,7 +664,7 @@ void gfc_restore_sym (gfc_symbol *, gfc_saved_var *);
void gfc_set_decl_assembler_name (tree, tree);
/* Returns true if a variable of specified size should go on the stack. */
-int gfc_can_put_var_on_stack (tree);
+bool gfc_can_put_var_on_stack (tree);
/* Set GFC_DECL_SCALAR_* on decl from sym if needed. */
void gfc_finish_decl_attrs (tree, symbol_attribute *);
--
2.38.1
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH 4/5] value-range: Add as_string diagnostics helper
2022-11-12 23:45 [PATCH 0/5] function result decl location; type demotion Bernhard Reutner-Fischer
` (2 preceding siblings ...)
2022-11-12 23:45 ` [PATCH 3/5] Fortran: Narrow return types [PR78798] Bernhard Reutner-Fischer
@ 2022-11-12 23:45 ` Bernhard Reutner-Fischer
2022-11-12 23:55 ` Andrew Pinski
2022-11-12 23:45 ` [PATCH 5/5] gimple: Add pass to note possible type demotions; IPA pro/demotion; DO NOT MERGE Bernhard Reutner-Fischer
4 siblings, 1 reply; 34+ messages in thread
From: Bernhard Reutner-Fischer @ 2022-11-12 23:45 UTC (permalink / raw)
To: gcc-patches
Cc: Bernhard Reutner-Fischer, Bernhard Reutner-Fischer, fortran,
Andrew MacLeod, Aldy Hernandez
gcc/ChangeLog:
* value-range.cc (get_bound_with_infinite_markers): New static helper.
(irange::as_string): New definition.
* value-range.h: New declaration.
---
Provide means to print a value range to a newly allocated buffer.
The caller is responsible to free() the allocated memory.
Bootstrapped and regtested on x86_86-unknown-linux with no regressions.
Ok for trunk?
Cc: Andrew MacLeod <amacleod@redhat.com>
Cc: Aldy Hernandez <aldyh@redhat.com>
---
gcc/value-range.cc | 56 ++++++++++++++++++++++++++++++++++++++++++++++
gcc/value-range.h | 3 +++
2 files changed, 59 insertions(+)
diff --git a/gcc/value-range.cc b/gcc/value-range.cc
index a855aaf626c..51cd9a38d90 100644
--- a/gcc/value-range.cc
+++ b/gcc/value-range.cc
@@ -3099,6 +3099,62 @@ debug (const value_range &vr)
fprintf (stderr, "\n");
}
+/* Helper for irange::as_string(). Print a bound to an allocated buffer. */
+static char *
+get_bound_with_infinite_markers (tree bound)
+{
+ tree type = TREE_TYPE (bound);
+ wide_int type_min = wi::min_value (TYPE_PRECISION (type), TYPE_SIGN (type));
+ wide_int type_max = wi::max_value (TYPE_PRECISION (type), TYPE_SIGN (type));
+
+ if (INTEGRAL_TYPE_P (type)
+ && !TYPE_UNSIGNED (type)
+ && TREE_CODE (bound) == INTEGER_CST
+ && wi::to_wide (bound) == type_min
+ && TYPE_PRECISION (type) != 1)
+ return xstrdup ("-INF");
+ else if (TREE_CODE (bound) == INTEGER_CST
+ && wi::to_wide (bound) == type_max
+ && TYPE_PRECISION (type) != 1)
+ return xstrdup ("+INF");
+ else
+ return print_generic_expr_to_str (bound);
+}
+
+
+/* Return an irange as string. Return NULL on failure, an allocated
+ string on success. */
+char *
+irange::as_string ()
+{
+ char *ret = NULL;
+ if (undefined_p() || varying_p () || m_num_ranges == 0)
+ return ret;
+
+ for (unsigned i = 0; i < m_num_ranges; ++i)
+ {
+ tree lb = m_base[i * 2];
+ tree ub = m_base[i * 2 + 1];
+ /* Construct [lower_bound,upper_bound]. */
+ char *lbs = get_bound_with_infinite_markers (lb);
+ char *ubs = get_bound_with_infinite_markers (ub);
+ /* Paranoia mode */
+ if (!lbs)
+ lbs = xstrdup ("");
+ if (!ubs)
+ ubs = xstrdup ("");
+
+ if (ret)
+ ret = reconcat (ret, ret, "[", lbs, ",", ubs, "]", NULL);
+ else
+ ret = concat ("[", lbs, ",", ubs, "]", NULL);
+
+ free (lbs);
+ free (ubs);
+ }
+ return ret;
+}
+
/* Create two value-ranges in *VR0 and *VR1 from the anti-range *AR
so that *VR0 U *VR1 == *AR. Returns true if that is possible,
false otherwise. If *AR can be represented with a single range
diff --git a/gcc/value-range.h b/gcc/value-range.h
index c87734dd8cd..76242e4bf45 100644
--- a/gcc/value-range.h
+++ b/gcc/value-range.h
@@ -160,6 +160,9 @@ public:
wide_int get_nonzero_bits () const;
void set_nonzero_bits (const wide_int_ref &bits);
+ // For diagnostics.
+ char *as_string ();
+
// Deprecated legacy public methods.
tree min () const; // DEPRECATED
tree max () const; // DEPRECATED
--
2.38.1
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH 5/5] gimple: Add pass to note possible type demotions; IPA pro/demotion; DO NOT MERGE
2022-11-12 23:45 [PATCH 0/5] function result decl location; type demotion Bernhard Reutner-Fischer
` (3 preceding siblings ...)
2022-11-12 23:45 ` [PATCH 4/5] value-range: Add as_string diagnostics helper Bernhard Reutner-Fischer
@ 2022-11-12 23:45 ` Bernhard Reutner-Fischer
4 siblings, 0 replies; 34+ messages in thread
From: Bernhard Reutner-Fischer @ 2022-11-12 23:45 UTC (permalink / raw)
To: gcc-patches; +Cc: Bernhard Reutner-Fischer, Bernhard Reutner-Fischer, fortran
gcc/ChangeLog:
* Makefile.in (OBJS): Add gimple-warn-types.o.
* passes.def: Add pass_warn_type_demotion.
* tree-pass.h (make_pass_warn_type_demotion): New declaration.
* gimple-warn-types.cc: New file.
gcc/c-family/ChangeLog:
* c.opt (Wtype-demotion): New.
---
DO NOT MERGE.
This is the script^Wpass to emit a warning if a function's return type
could potentially be narrowed.
What would probably be useful is to equip an IPA pass with that
knowledge and demote return types of functions that do not contribute to
an external interface to the smallest possible type. The idea is that if
a target determines late via targetm.calls.promote_prototypes to promote
return values, the target will ultimately have the final say about
return types while for non-exported functions we can narrow types for
size- or speed reasons as we see fit.
This hunk does not implement an IPA pass that would do anything really
useful but merely queries the ranger to see if there are possibilities
to demote a return type, any (!) return type.
For the IPA real thing, we'd want to notice that if a function returns a
singleton, the caller would just use that singleton and demote the
callee to void. And in the caller, we'd appropriately shift the callee's
return value to the required range/value.
The usual trouble makers come to mind: qsort helpers that insist on int
return codes (that we could extend to int).
As said, that's just for your amusement and is not meant to be merged.
---
gcc/Makefile.in | 1 +
gcc/c-family/c.opt | 6 +
gcc/gimple-warn-types.cc | 441 +++++++++++++++++++++++++++++++++++++++
gcc/passes.def | 1 +
gcc/tree-pass.h | 1 +
5 files changed, 450 insertions(+)
create mode 100644 gcc/gimple-warn-types.cc
diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index f672e6ea549..c6901ececd4 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -1433,6 +1433,7 @@ OBJS = \
gimple-streamer-out.o \
gimple-walk.o \
gimple-warn-recursion.o \
+ gimple-warn-types.o \
gimplify.o \
gimplify-me.o \
godump.o \
diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 63a300ecd7c..0b46669e2b7 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -1005,6 +1005,12 @@ Wtemplates
C++ ObjC++ Var(warn_templates) Warning
Warn on primary template declaration.
+Wtype-demotion
+C ObjC C++ ObjC++ Var(warn_type_demotion) Warning LangEnabledBy(C ObjC C++ ObjC++, Wall)
+Warn if function return type could be narrowed or demoted.
+; function return type, parameter type, variable type.
+; if values used for a type indicate that the type could use a narrower mode.
+
Wmissing-attributes
C ObjC C++ ObjC++ Var(warn_missing_attributes) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall)
Warn about declarations of entities that may be missing attributes
diff --git a/gcc/gimple-warn-types.cc b/gcc/gimple-warn-types.cc
new file mode 100644
index 00000000000..e0b7212a1bb
--- /dev/null
+++ b/gcc/gimple-warn-types.cc
@@ -0,0 +1,441 @@
+/* Pass to detect and issue warnings about possibly using narrower types.
+
+ Copyright (C) 2021-2022 Free Software Foundation, Inc.
+ Contributed by Bernhard Reutner-Fischer <aldot@gcc.gnu.org>
+
+ This file is part of GCC.
+
+ GCC is free software; you can redistribute it and/or modify it under
+ the terms of the GNU General Public License as published by the Free
+ Software Foundation; either version 3, or (at your option) any later
+ version.
+
+ GCC is distributed in the hope that it will be useful, but WITHOUT ANY
+ WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with GCC; see the file COPYING3. If not see
+ <http://www.gnu.org/licenses/>. */
+
+#include "config.h"
+#include "system.h"
+#include "coretypes.h"
+#include "backend.h"
+#include "tree.h"
+#include "gimple.h"
+#include "tree-pass.h"
+#include "pointer-query.h"
+#include "ssa.h"
+#include "gimple-pretty-print.h"
+#include "diagnostic-core.h"
+#include "fold-const.h"
+#include "gimple-iterator.h"
+#include "tree-dfa.h" //
+#include "tree-ssa.h"
+#include "tree-cfg.h"
+#include "tree-object-size.h" //
+#include "calls.h" //
+#include "cfgloop.h" //
+#include "intl.h"
+#include "gimple-range.h"
+
+#include "value-range.h"
+#include "gimple-range-path.h"
+#include "gcc-rich-location.h"
+#include "langhooks.h"
+#pragma GCC optimize("O0")
+namespace {
+
+const pass_data pass_data_wtype_demotion = {
+ GIMPLE_PASS,
+ "wtype_demotion",
+ OPTGROUP_NONE,
+ TV_NONE,
+ PROP_cfg, /* properties_required. */
+ 0, /* properties_provided. */
+ 0, /* properties_destroyed. */
+ 0, /* properties_start */
+ 0, /* properties_finish */
+};
+
+class pass_wtype_demotion : public gimple_opt_pass
+{
+ public:
+ pass_wtype_demotion (gcc::context *ctxt)
+ : gimple_opt_pass (pass_data_wtype_demotion, ctxt)
+ { }
+
+ opt_pass *clone () { return new pass_wtype_demotion (m_ctxt); }
+
+ virtual bool gate (function *);
+ virtual unsigned int execute (function *);
+};
+
+bool
+pass_wtype_demotion::gate (function *)
+{
+ return warn_type_demotion;
+}
+
+} /* anonymous namespace */
+
+gimple_opt_pass *
+make_pass_warn_type_demotion (gcc::context *ctxt)
+{
+ return new pass_wtype_demotion (ctxt);
+}
+
+\f
+
+/* Determine the smallest type capable of representing values between TMIN
+ and TMAX with sign TSGN. Returns a demoted type. */
+static tree
+narrowest_type (wide_int tmin, wide_int tmax, signop tsgn)
+{
+ tree t;
+ gcc_checking_assert (wi::cmp(tmax, tmin, tsgn) >= 0);
+ const unsigned int tmin_bits = wi::min_precision (tmin, tsgn);
+ const unsigned int tmax_bits = wi::min_precision (tmax, tsgn);
+ const bool equal_sgn = wi::neg_p (tmin, tsgn) == wi::neg_p (tmax, tsgn);
+ const unsigned int bp = wi::get_binary_precision (tmin, tmax);
+ const bool fitsl = wi::fits_uhwi_p (tmin);
+ const bool fitsh = wi::fits_uhwi_p (tmax);
+ int fl = wi::floor_log2 (tmin);
+ int fh = wi::floor_log2 (tmax);
+ signop sgn;
+ unsigned int bits;
+
+ /* ideally equal_sgn [46,122] (i.e. [101110,1111010]) -> [0,76] 6bit, << 1 */
+ if (equal_sgn)
+ bits = tmax_bits - tmin_bits == 1 ? tmin_bits : fh - fl;
+ //bits = fh - fl;
+ else
+ bits = tmin_bits + tmax_bits;
+ bits = MIN (bp, bits);
+
+ if (bits <= 1)
+ t = boolean_type_node; // build_nonstandard_boolean_type (bits);
+ else
+ {
+ if (equal_sgn && fitsl && fitsh && bits < HOST_BITS_PER_WIDE_INT)
+ sgn = UNSIGNED;
+ else
+ sgn = tsgn;
+
+ t = lang_hooks.types.type_for_size (bits, sgn == UNSIGNED);
+#if 0
+ The nonstandard type would not have a typename to suggest, skip it.
+ if (t == NULL_TREE)
+ {
+ /* Probably a type with > 64 bits. */
+ /* We currently disregard complex or float types. */
+ scalar_int_mode m = smallest_int_mode_for_size (bits);
+ t = build_nonstandard_integer_type (bits, sgn == UNSIGNED);
+ }
+#endif
+ }
+ return t;
+}
+
+/* A bit like gcc/tree-cfg.cc last_stmt(), but for a whole function. */
+/* Return the last nondebug statement of function FUN. */
+
+static gimple *
+fn_last_stmt (function *fun)
+{
+ basic_block bb = EXIT_BLOCK_PTR_FOR_FN (fun);
+ gimple_stmt_iterator gsi;
+ do {
+ bb = bb->prev_bb;
+ gsi = gsi_last_nondebug_bb (bb);
+ } while (gsi_end_p (gsi) && bb != ENTRY_BLOCK_PTR_FOR_FN (fun));
+ if (gsi_end_p (gsi))
+ /* There were no real statements whatsoever. */
+ return NULL;
+ return gsi_stmt (gsi);
+}
+
+/* Return true if the type of RESULT_TYPE of the functions result_decl is
+ handled, false otherwise. */
+
+static bool
+handled_result_decl_type_p (tree result_type)
+{
+ switch (TREE_CODE (result_type))
+ {
+ case VOID_TYPE:
+ case ERROR_MARK:
+ case COMPLEX_TYPE: /* Don't care. */
+ case REAL_TYPE: /* Don't care. */
+ case ENUMERAL_TYPE: /* Probably does not make sense to mention. */
+ /* Should maybe look through and suggest to adjust the innards */
+ case RECORD_TYPE:
+ case UNION_TYPE:
+ return false;
+ default: return true;
+ }
+}
+
+/* Return true if the gimple STMT is handled.
+ If it is, set VR to the value_range by querying ranger R.
+ Return false otherwise. */
+
+static bool
+handled_result_decl_code_p (gimple_ranger *r, gimple *stmt, value_range &vr)
+{
+ bool ok = false;
+ tree tstmt;
+
+ switch (gimple_code (stmt))
+ {
+ case GIMPLE_ASM:
+ case GIMPLE_NOP:
+ break;
+ case GIMPLE_RETURN:
+ tstmt = gimple_return_retval (as_a <greturn *> (stmt));
+ /* c-c++-common/pr53633-2.c, naked function
+ (gdb) call debug_gimple_stmt(stmt)
+ # VUSE <.MEM_1(D)>
+ return;
+ p tstmt
+ $3 = <tree 0x0>
+ */
+ if (tstmt == NULL_TREE)
+ return ok;
+ ok = r->range_of_expr (vr, tstmt,
+ as_a <greturn *> (stmt));
+ break;
+ case GIMPLE_ASSIGN:
+#if 0
+ /* libgomp/loop_ull.c:414 gomp_loop_ull_dynamic_start
+(gdb) call debug(bb)
+<bb 45> [local count: 31004295]:
+_159 = team_45->nthreads;
+iftmp.1_87 = (long int) _159;
+_91 = (long long unsigned int) iftmp.1_87;
+# DEBUG nthreads => iftmp.1_87
+# DEBUG BEGIN_STMT
+goto <bb 17>; [100.00%]
+
+(gdb) p gsi_stmt(gsi_last_nondebug_bb (bb))
+$66 = <gimple_assign 0x7ffff6ef1cd0>
+(gdb) call debug(gsi_stmt(gsi_last_nondebug_bb (bb)))
+_91 = (long long unsigned int) iftmp.1_87;
+ The goto is the successor block flag
+ */
+ ok = get_range_query (fun)->range_of_expr (vr,
+ gimple_assign_lhs (as_a <gassign *> (stmt)),
+ as_a <gassign *> (stmt));
+#endif
+ break;
+ case GIMPLE_COND:
+#if 0
+ /* libgcc/generic-morestack.c:414 __generic_morestack */
+ ok = get_range_query (fun)->range_of_expr (vr,
+ gimple_cond_lhs (as_a <gcond *> (stmt)),
+ as_a <gcond *> (stmt));
+#endif
+ break;
+ case GIMPLE_CALL:
+#if 0
+ ok = get_range_query (fun)->range_of_expr (vr,
+ gimple_call_lhs (as_a <gcall *> (stmt)),
+ as_a <gcall *> (stmt));
+#endif
+ break;
+ case GIMPLE_SWITCH:
+#if 0
+ /* combine.c simplify_shift_const_1 */
+ break;
+#endif
+ case GIMPLE_LABEL:
+ break;
+ case GIMPLE_PHI:
+ case GIMPLE_RESX:
+ break;
+ default:
+ gcc_unreachable ();
+ }
+ return ok;
+}
+
+static unsigned
+wnarrow_return (gimple_ranger *ranger, function *fun, bool do_demotion=false)
+{
+// targetm.calls.promote_prototypes
+// current_function_return_value
+// outer_curly_brace_block
+
+ tree decl = fun->decl;
+ /* Leave main alone. */
+ if (MAIN_NAME_P (DECL_NAME (decl)))
+ return false;
+
+ /* We should probably leave the type of an exported API alone.. */
+ if (00 && DECL_VISIBILITY_SPECIFIED (decl)
+ && DECL_VISIBILITY (decl) == VISIBILITY_DEFAULT)
+ return false;
+
+ tree result = DECL_RESULT (decl);
+ tree result_type = TREE_TYPE (result);
+
+ if (! handled_result_decl_type_p (result_type))
+ return false;
+
+ /* Get the last nondebug statement of function FUN. */
+ gimple *stmt = fn_last_stmt (fun);
+ if (stmt == NULL)
+ return false;
+
+ /* See if we can handle the gimple stmt to deduce a value_range. */
+ value_range vr;
+ if (! handled_result_decl_code_p (ranger, stmt, vr))
+ return false;
+
+ if (vr.undefined_p())
+ return false; // Should not occur when returning ints
+ if (vr.varying_p())
+ return false; // Cannot do much about those
+
+ location_t loc = DECL_SOURCE_LOCATION (result);
+ gcc_rich_location iloc (loc);
+
+ if (!warnings_are_errors && vr.singleton_p() && 00) /* disabled for now */
+ {
+ auto_diagnostic_group d;
+ iloc.add_fixit_replace ("void");
+ // borked iloc.add_fixit_remove (LOCATION_LOCUS (gimple_location (stmt)));
+ if (warning_at (&iloc, OPT_Wtype_demotion,
+ "Function %qE returns a single value", DECL_NAME(decl)))
+ {
+ if (vr.num_pairs () == 1 && vr.lower_bound () == vr.upper_bound ())
+ {
+ if (wi::fits_shwi_p (vr.lower_bound ()))
+ inform (loc, "Maybe the function should be %<void%> "
+ "and the caller use a const value of "
+ "%wd "
+ "instead of the result of the call",
+ vr.lower_bound ().to_shwi ());
+ else
+ inform (loc, "Maybe the function should be %<void%> "
+ "and the caller use a const value of "
+ "%wu "
+ "instead of the result of the call",
+ vr.lower_bound ().to_uhwi ());
+ }
+ else
+ {
+ gcc_unreachable (); // TODO: do this for non-singletons
+ char *range_str = vr.as_string ();
+ if (range_str)
+ inform (loc, "Maybe the function should be %<void%> "
+ "and the caller use a fixed value in the range of %s "
+ "instead of the result of the call",
+ range_str);
+ free (range_str);
+ }
+ }
+ return false;
+ }
+
+ wide_int ret_min = vr.lower_bound();
+ wide_int ret_max = vr.upper_bound();
+ signop ret_sgn = TYPE_SIGN (vr.type ());
+
+ // TODO: Nah. We want [-42,-1] to suggest unsigned char
+ // TODO: and shift the caller accordingly.
+ if (00 && ! wi::neg_p (ret_min, ret_sgn) && ! wi::neg_p (ret_max, ret_sgn))
+ ret_sgn = UNSIGNED;
+
+ tree demoted = narrowest_type (ret_min, ret_max, ret_sgn);
+ if (demoted == NULL_TREE)
+ /* libgcc2.c UDWtype __fixunssfDI (SFtype a) , 128bit. */
+ /* Don't know which typename to suggest to the user.. typeid? Punt. */
+ return false;
+ gcc_checking_assert (demoted != NULL_TREE);
+//int o = TYPE_PRECISION (result_type);
+//int n = TYPE_PRECISION (demoted);
+ gcc_checking_assert (TYPE_PRECISION (demoted)
+ <= TYPE_PRECISION (result_type));
+
+ /* Punt if we cannot reduce the number of bits required.
+ ??? should we still favour signed -> unsigned conversion if possible?
+ */
+ if (TYPE_PRECISION (demoted) == TYPE_PRECISION (result_type))
+ return false;
+
+ if (warnings_are_errors)
+ return false; // libgomp/icv.c
+
+ char *caller_adjust = NULL;
+ int diff = 0;
+ if (TYPE_SIGN (demoted) != TYPE_SIGN (result_type))
+ {
+ if (TYPE_SIGN (demoted) == UNSIGNED)
+ {
+ /* TYPE_MIN_VALUE (demoted) == 0 */
+ //diff = wi::abs (wi::to_wide (vr.min ()));
+ diff = tree_int_cst_sign_bit (vr.min ());
+ caller_adjust = xasprintf ("+ %d", diff);
+ }
+ else
+ {
+ diff = tree_int_cst_sign_bit (vr.max ())
+ - tree_int_cst_sign_bit (vr.min ());
+ caller_adjust = xasprintf ("XXX %d", diff);
+ }
+ }
+ auto_diagnostic_group d;
+ iloc.add_fixit_replace (
+ DECL_SOURCE_LOCATION (result),
+ IDENTIFIER_POINTER (DECL_NAME (TYPE_NAME (demoted))));
+ if (warning_at (&iloc, OPT_Wtype_demotion, "Function %qE could return %qE",
+ DECL_NAME (decl), DECL_NAME (TYPE_NAME (demoted))))
+ {
+ char *range_str = vr.as_string ();
+ if (range_str)
+ {
+ if (diff)
+ inform (loc, "with a range of %s and %s in the caller", range_str,
+ caller_adjust);
+ else
+ inform (loc, "with a range of %s", range_str);
+ }
+ free (range_str);
+ }
+ free (caller_adjust);
+ if (dump_file)
+ {
+ fprintf (dump_file, "Function %s return type:\nold %s\nnew %s\n",
+ print_generic_expr_to_str (DECL_NAME (decl)),
+ print_generic_expr_to_str (DECL_NAME (result_type)),
+ print_generic_expr_to_str (DECL_NAME (TYPE_NAME (demoted))));
+ }
+
+ if (!do_demotion)
+ return false;
+
+ /* We have a demoted type to use for the function result, use it. */
+#if 1
+ return false;
+#else
+ //TREE_TYPE (result) = demoted;
+ //*result_type = *demoted;
+ This would have to be an IPA pass
+ return true;
+#endif
+}
+
+unsigned
+pass_wtype_demotion::execute (function *fun)
+{
+ gimple_ranger ranger;
+
+ if (wnarrow_return (&ranger, fun, /*do_demotion=*/true))
+ return TODO_update_ssa;
+
+ return 0;
+}
+
diff --git a/gcc/passes.def b/gcc/passes.def
index 193b5794749..d6c4a0609f5 100644
--- a/gcc/passes.def
+++ b/gcc/passes.def
@@ -202,6 +202,7 @@ along with GCC; see the file COPYING3. If not see
form if possible. */
NEXT_PASS (pass_object_sizes);
NEXT_PASS (pass_post_ipa_warn);
+ NEXT_PASS (pass_warn_type_demotion);
/* Must run before loop unrolling. */
NEXT_PASS (pass_warn_access, /*early=*/true);
NEXT_PASS (pass_complete_unrolli);
diff --git a/gcc/tree-pass.h b/gcc/tree-pass.h
index 8480d41384b..5d314bc7865 100644
--- a/gcc/tree-pass.h
+++ b/gcc/tree-pass.h
@@ -480,6 +480,7 @@ extern gimple_opt_pass *make_pass_local_pure_const (gcc::context *ctxt);
extern gimple_opt_pass *make_pass_nothrow (gcc::context *ctxt);
extern gimple_opt_pass *make_pass_tracer (gcc::context *ctxt);
extern gimple_opt_pass *make_pass_warn_restrict (gcc::context *ctxt);
+extern gimple_opt_pass *make_pass_warn_type_demotion (gcc::context *ctxt);
extern gimple_opt_pass *make_pass_warn_unused_result (gcc::context *ctxt);
extern gimple_opt_pass *make_pass_diagnose_tm_blocks (gcc::context *ctxt);
extern gimple_opt_pass *make_pass_lower_tm (gcc::context *ctxt);
--
2.38.1
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 4/5] value-range: Add as_string diagnostics helper
2022-11-12 23:45 ` [PATCH 4/5] value-range: Add as_string diagnostics helper Bernhard Reutner-Fischer
@ 2022-11-12 23:55 ` Andrew Pinski
2022-11-17 3:30 ` Jeff Law
0 siblings, 1 reply; 34+ messages in thread
From: Andrew Pinski @ 2022-11-12 23:55 UTC (permalink / raw)
To: Bernhard Reutner-Fischer
Cc: gcc-patches, Bernhard Reutner-Fischer, fortran, Andrew MacLeod,
Aldy Hernandez
On Sat, Nov 12, 2022 at 3:47 PM Bernhard Reutner-Fischer via
Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
>
> gcc/ChangeLog:
>
> * value-range.cc (get_bound_with_infinite_markers): New static helper.
> (irange::as_string): New definition.
> * value-range.h: New declaration.
>
> ---
> Provide means to print a value range to a newly allocated buffer.
> The caller is responsible to free() the allocated memory.
>
> Bootstrapped and regtested on x86_86-unknown-linux with no regressions.
> Ok for trunk?
>
> Cc: Andrew MacLeod <amacleod@redhat.com>
> Cc: Aldy Hernandez <aldyh@redhat.com>
> ---
> gcc/value-range.cc | 56 ++++++++++++++++++++++++++++++++++++++++++++++
> gcc/value-range.h | 3 +++
> 2 files changed, 59 insertions(+)
>
> diff --git a/gcc/value-range.cc b/gcc/value-range.cc
> index a855aaf626c..51cd9a38d90 100644
> --- a/gcc/value-range.cc
> +++ b/gcc/value-range.cc
> @@ -3099,6 +3099,62 @@ debug (const value_range &vr)
> fprintf (stderr, "\n");
> }
>
> +/* Helper for irange::as_string(). Print a bound to an allocated buffer. */
> +static char *
Can we start using std::string instead of char* here?
> +get_bound_with_infinite_markers (tree bound)
> +{
> + tree type = TREE_TYPE (bound);
> + wide_int type_min = wi::min_value (TYPE_PRECISION (type), TYPE_SIGN (type));
> + wide_int type_max = wi::max_value (TYPE_PRECISION (type), TYPE_SIGN (type));
> +
> + if (INTEGRAL_TYPE_P (type)
> + && !TYPE_UNSIGNED (type)
> + && TREE_CODE (bound) == INTEGER_CST
> + && wi::to_wide (bound) == type_min
> + && TYPE_PRECISION (type) != 1)
> + return xstrdup ("-INF");
> + else if (TREE_CODE (bound) == INTEGER_CST
> + && wi::to_wide (bound) == type_max
> + && TYPE_PRECISION (type) != 1)
> + return xstrdup ("+INF");
> + else
> + return print_generic_expr_to_str (bound);
No reason to do xstrdup any more either.
> +}
> +
> +
> +/* Return an irange as string. Return NULL on failure, an allocated
> + string on success. */
> +char *
Likewise.
Thanks,
Andrew Pinski
> +irange::as_string ()
> +{
> + char *ret = NULL;
This becomes std::string ret;
> + if (undefined_p() || varying_p () || m_num_ranges == 0)
> + return ret;
> +
> + for (unsigned i = 0; i < m_num_ranges; ++i)
> + {
> + tree lb = m_base[i * 2];
> + tree ub = m_base[i * 2 + 1];
> + /* Construct [lower_bound,upper_bound]. */
> + char *lbs = get_bound_with_infinite_markers (lb);
> + char *ubs = get_bound_with_infinite_markers (ub);
> + /* Paranoia mode */
> + if (!lbs)
> + lbs = xstrdup ("");
> + if (!ubs)
> + ubs = xstrdup ("");
> +
> + if (ret)
> + ret = reconcat (ret, ret, "[", lbs, ",", ubs, "]", NULL);
> + else
> + ret = concat ("[", lbs, ",", ubs, "]", NULL);
> +
> + free (lbs);
> + free (ubs);
> + }
> + return ret;
> +}
> +
> /* Create two value-ranges in *VR0 and *VR1 from the anti-range *AR
> so that *VR0 U *VR1 == *AR. Returns true if that is possible,
> false otherwise. If *AR can be represented with a single range
> diff --git a/gcc/value-range.h b/gcc/value-range.h
> index c87734dd8cd..76242e4bf45 100644
> --- a/gcc/value-range.h
> +++ b/gcc/value-range.h
> @@ -160,6 +160,9 @@ public:
> wide_int get_nonzero_bits () const;
> void set_nonzero_bits (const wide_int_ref &bits);
>
> + // For diagnostics.
> + char *as_string ();
> +
> // Deprecated legacy public methods.
> tree min () const; // DEPRECATED
> tree max () const; // DEPRECATED
> --
> 2.38.1
>
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 3/5] Fortran: Narrow return types [PR78798]
2022-11-12 23:45 ` [PATCH 3/5] Fortran: Narrow return types [PR78798] Bernhard Reutner-Fischer
@ 2022-11-13 10:13 ` Janne Blomqvist
2022-11-13 10:39 ` Bernhard Reutner-Fischer
0 siblings, 1 reply; 34+ messages in thread
From: Janne Blomqvist @ 2022-11-13 10:13 UTC (permalink / raw)
To: Bernhard Reutner-Fischer; +Cc: gcc-patches, Bernhard Reutner-Fischer, fortran
On Sun, Nov 13, 2022 at 1:47 AM Bernhard Reutner-Fischer via Fortran
<fortran@gcc.gnu.org> wrote:
> --- a/gcc/fortran/arith.cc
> +++ b/gcc/fortran/arith.cc
> @@ -1135,7 +1135,7 @@ compare_complex (gfc_expr *op1, gfc_expr *op2)
> strings. We return -1 for a < b, 0 for a == b and 1 for a > b.
> We use the processor's default collating sequence. */
>
> -int
> +signed char
> gfc_compare_string (gfc_expr *a, gfc_expr *b)
> {
> size_t len, alen, blen, i;
> @@ -1162,7 +1162,7 @@ gfc_compare_string (gfc_expr *a, gfc_expr *b)
> }
Hmm, really? PR 78798 mentions changing int to bool, where
appropriate, which I think is uncontroversial, but this?
--
Janne Blomqvist
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 3/5] Fortran: Narrow return types [PR78798]
2022-11-13 10:13 ` Janne Blomqvist
@ 2022-11-13 10:39 ` Bernhard Reutner-Fischer
2022-11-13 20:29 ` Harald Anlauf
0 siblings, 1 reply; 34+ messages in thread
From: Bernhard Reutner-Fischer @ 2022-11-13 10:39 UTC (permalink / raw)
To: Janne Blomqvist
Cc: rep.dot.nop, gcc-patches, Bernhard Reutner-Fischer, fortran
On Sun, 13 Nov 2022 12:13:26 +0200
Janne Blomqvist <blomqvist.janne@gmail.com> wrote:
> On Sun, Nov 13, 2022 at 1:47 AM Bernhard Reutner-Fischer via Fortran
> <fortran@gcc.gnu.org> wrote:
> > --- a/gcc/fortran/arith.cc
> > +++ b/gcc/fortran/arith.cc
> > @@ -1135,7 +1135,7 @@ compare_complex (gfc_expr *op1, gfc_expr *op2)
> > strings. We return -1 for a < b, 0 for a == b and 1 for a > b.
> > We use the processor's default collating sequence. */
> >
> > -int
> > +signed char
> > gfc_compare_string (gfc_expr *a, gfc_expr *b)
> > {
> > size_t len, alen, blen, i;
> > @@ -1162,7 +1162,7 @@ gfc_compare_string (gfc_expr *a, gfc_expr *b)
> > }
>
> Hmm, really? PR 78798 mentions changing int to bool, where
> appropriate, which I think is uncontroversial, but this?
Well we could leave this or all spots alone where a bool is
insufficient, if you prefer.
In the case of gfc_compare_string, the only user is simplify which only
looks at ge/gt/le/lt 0
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 3/5] Fortran: Narrow return types [PR78798]
2022-11-13 10:39 ` Bernhard Reutner-Fischer
@ 2022-11-13 20:29 ` Harald Anlauf
2022-11-13 20:29 ` Harald Anlauf
2022-11-13 21:50 ` Bernhard Reutner-Fischer
0 siblings, 2 replies; 34+ messages in thread
From: Harald Anlauf @ 2022-11-13 20:29 UTC (permalink / raw)
To: Bernhard Reutner-Fischer, Janne Blomqvist
Cc: gcc-patches, Bernhard Reutner-Fischer, fortran
Am 13.11.22 um 11:39 schrieb Bernhard Reutner-Fischer via Gcc-patches:
> On Sun, 13 Nov 2022 12:13:26 +0200
> Janne Blomqvist <blomqvist.janne@gmail.com> wrote:
>
>> On Sun, Nov 13, 2022 at 1:47 AM Bernhard Reutner-Fischer via Fortran
>> <fortran@gcc.gnu.org> wrote:
>>> --- a/gcc/fortran/arith.cc
>>> +++ b/gcc/fortran/arith.cc
>>> @@ -1135,7 +1135,7 @@ compare_complex (gfc_expr *op1, gfc_expr *op2)
>>> strings. We return -1 for a < b, 0 for a == b and 1 for a > b.
>>> We use the processor's default collating sequence. */
>>>
>>> -int
>>> +signed char
>>> gfc_compare_string (gfc_expr *a, gfc_expr *b)
>>> {
>>> size_t len, alen, blen, i;
>>> @@ -1162,7 +1162,7 @@ gfc_compare_string (gfc_expr *a, gfc_expr *b)
>>> }
>>
>> Hmm, really? PR 78798 mentions changing int to bool, where
>> appropriate, which I think is uncontroversial, but this?
>
> Well we could leave this or all spots alone where a bool is
> insufficient, if you prefer.
>
> In the case of gfc_compare_string, the only user is simplify which only
> looks at ge/gt/le/lt 0
My reading of the mentioned PR is that there is a fundamental
disagreement with the subject:
Bug 78798 - [cleanup] some int-valued functions should be bool
I see that as an issue of (a minor lack of) conciseness;
it is *not* about narrowing.
Replacing "int" by "signed char" adds confusion and makes code
less understandable, so I would oppose it, as we don't solve a
real problem and rather add confusion.
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 3/5] Fortran: Narrow return types [PR78798]
2022-11-13 20:29 ` Harald Anlauf
@ 2022-11-13 20:29 ` Harald Anlauf
2022-11-13 21:50 ` Bernhard Reutner-Fischer
1 sibling, 0 replies; 34+ messages in thread
From: Harald Anlauf @ 2022-11-13 20:29 UTC (permalink / raw)
To: gcc-patches; +Cc: fortran
Am 13.11.22 um 11:39 schrieb Bernhard Reutner-Fischer via Gcc-patches:
> On Sun, 13 Nov 2022 12:13:26 +0200
> Janne Blomqvist <blomqvist.janne@gmail.com> wrote:
>
>> On Sun, Nov 13, 2022 at 1:47 AM Bernhard Reutner-Fischer via Fortran
>> <fortran@gcc.gnu.org> wrote:
>>> --- a/gcc/fortran/arith.cc
>>> +++ b/gcc/fortran/arith.cc
>>> @@ -1135,7 +1135,7 @@ compare_complex (gfc_expr *op1, gfc_expr *op2)
>>> strings. We return -1 for a < b, 0 for a == b and 1 for a > b.
>>> We use the processor's default collating sequence. */
>>>
>>> -int
>>> +signed char
>>> gfc_compare_string (gfc_expr *a, gfc_expr *b)
>>> {
>>> size_t len, alen, blen, i;
>>> @@ -1162,7 +1162,7 @@ gfc_compare_string (gfc_expr *a, gfc_expr *b)
>>> }
>>
>> Hmm, really? PR 78798 mentions changing int to bool, where
>> appropriate, which I think is uncontroversial, but this?
>
> Well we could leave this or all spots alone where a bool is
> insufficient, if you prefer.
>
> In the case of gfc_compare_string, the only user is simplify which only
> looks at ge/gt/le/lt 0
My reading of the mentioned PR is that there is a fundamental
disagreement with the subject:
Bug 78798 - [cleanup] some int-valued functions should be bool
I see that as an issue of (a minor lack of) conciseness;
it is *not* about narrowing.
Replacing "int" by "signed char" adds confusion and makes code
less understandable, so I would oppose it, as we don't solve a
real problem and rather add confusion.
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 3/5] Fortran: Narrow return types [PR78798]
2022-11-13 20:29 ` Harald Anlauf
2022-11-13 20:29 ` Harald Anlauf
@ 2022-11-13 21:50 ` Bernhard Reutner-Fischer
2023-05-10 16:47 ` [PATCH v2] " Bernhard Reutner-Fischer
1 sibling, 1 reply; 34+ messages in thread
From: Bernhard Reutner-Fischer @ 2022-11-13 21:50 UTC (permalink / raw)
To: Harald Anlauf, Janne Blomqvist
Cc: gcc-patches, Bernhard Reutner-Fischer, fortran
On 13 November 2022 21:29:50 CET, Harald Anlauf <anlauf@gmx.de> wrote:
>Replacing "int" by "signed char" adds confusion and makes code
>less understandable, so I would oppose it, as we don't solve a
>real problem and rather add confusion.
Ok so consider the non-bool hunks dropped, they just fell out of my helper and I thought I'd ask.
I can send an updated patch during the weekend.
thanks,
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 1/5] c: Set the locus of the function result decl
2022-11-12 23:45 ` [PATCH 1/5] c: Set the locus of the function result decl Bernhard Reutner-Fischer
@ 2022-11-14 21:25 ` Joseph Myers
0 siblings, 0 replies; 34+ messages in thread
From: Joseph Myers @ 2022-11-14 21:25 UTC (permalink / raw)
To: Bernhard Reutner-Fischer; +Cc: gcc-patches, Bernhard Reutner-Fischer, fortran
On Sun, 13 Nov 2022, Bernhard Reutner-Fischer via Gcc-patches wrote:
> Bootstrapped and regtested on x86_86-unknown-linux with no regressions.
> Ok for trunk?
>
> Cc: Joseph Myers <joseph@codesourcery.com>
> ---
> gcc/c/ChangeLog:
>
> * c-decl.cc (start_function): Set the result decl source
> location to the location of the typespec.
OK.
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 2/5] c++: Set the locus of the function result decl
2022-11-12 23:45 ` [PATCH 2/5] c++: " Bernhard Reutner-Fischer
@ 2022-11-15 23:52 ` Jason Merrill
2022-11-17 8:56 ` Bernhard Reutner-Fischer
0 siblings, 1 reply; 34+ messages in thread
From: Jason Merrill @ 2022-11-15 23:52 UTC (permalink / raw)
To: Bernhard Reutner-Fischer, gcc-patches
Cc: Bernhard Reutner-Fischer, fortran, Nathan Sidwell
On 11/12/22 13:45, Bernhard Reutner-Fischer wrote:
> gcc/cp/ChangeLog:
>
> * decl.cc (start_function): Set the result decl source location to
> the location of the typespec.
>
> ---
> Bootstrapped and regtested on x86_86-unknown-linux with no regressions.
> Ok for trunk?
>
> Cc: Nathan Sidwell <nathan@acm.org>
> Cc: Jason Merrill <jason@redhat.com>
> ---
> gcc/cp/decl.cc | 15 ++++++++++++++-
> 1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
> index 6e98ea35a39..ed40815e645 100644
> --- a/gcc/cp/decl.cc
> +++ b/gcc/cp/decl.cc
> @@ -17449,6 +17449,8 @@ start_function (cp_decl_specifier_seq *declspecs,
> tree attrs)
> {
> tree decl1;
> + tree result;
> + bool ret;
We now prefer to declare new variables as late as possible, usually when
they are initialized.
> decl1 = grokdeclarator (declarator, declspecs, FUNCDEF, 1, &attrs);
> invoke_plugin_callbacks (PLUGIN_START_PARSE_FUNCTION, decl1);
> @@ -17461,7 +17463,18 @@ 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);
> + ret = start_preparsed_function (decl1, attrs, /*flags=*/SF_DEFAULT);
> +
> + /* decl1 might be ggc_freed here. */
> + decl1 = current_function_decl;
> +
> + /* 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];
One way to handle the template case would be for the code in
start_preparsed_function that sets DECL_RESULT to check whether decl1 is
a template instantiation, and in that case copy the location from the
template's DECL_RESULT, i.e.
DECL_RESULT (DECL_TEMPLATE_RESULT (DECL_TI_TEMPLATE (decl1)))
> + return ret;
> }
> \f
> /* Returns true iff an EH_SPEC_BLOCK should be created in the body of
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 4/5] value-range: Add as_string diagnostics helper
2022-11-12 23:55 ` Andrew Pinski
@ 2022-11-17 3:30 ` Jeff Law
0 siblings, 0 replies; 34+ messages in thread
From: Jeff Law @ 2022-11-17 3:30 UTC (permalink / raw)
To: Andrew Pinski, Bernhard Reutner-Fischer
Cc: gcc-patches, Bernhard Reutner-Fischer, fortran, Andrew MacLeod,
Aldy Hernandez
On 11/12/22 16:55, Andrew Pinski via Gcc-patches wrote:
> On Sat, Nov 12, 2022 at 3:47 PM Bernhard Reutner-Fischer via
> Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
>> gcc/ChangeLog:
>>
>> * value-range.cc (get_bound_with_infinite_markers): New static helper.
>> (irange::as_string): New definition.
>> * value-range.h: New declaration.
>>
>> ---
>> Provide means to print a value range to a newly allocated buffer.
>> The caller is responsible to free() the allocated memory.
>>
>> Bootstrapped and regtested on x86_86-unknown-linux with no regressions.
>> Ok for trunk?
>>
>> Cc: Andrew MacLeod <amacleod@redhat.com>
>> Cc: Aldy Hernandez <aldyh@redhat.com>
>> ---
>> gcc/value-range.cc | 56 ++++++++++++++++++++++++++++++++++++++++++++++
>> gcc/value-range.h | 3 +++
>> 2 files changed, 59 insertions(+)
>>
>> diff --git a/gcc/value-range.cc b/gcc/value-range.cc
>> index a855aaf626c..51cd9a38d90 100644
>> --- a/gcc/value-range.cc
>> +++ b/gcc/value-range.cc
>> @@ -3099,6 +3099,62 @@ debug (const value_range &vr)
>> fprintf (stderr, "\n");
>> }
>>
>> +/* Helper for irange::as_string(). Print a bound to an allocated buffer. */
>> +static char *
> Can we start using std::string instead of char* here?
If it makes the code easier to read/maintain, sure. std::string isn't
used heavily, but has crept into a few places, mostly in target files.
std::string isn't something we've pushed at all in terms of preferred
practices.
Jeff
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 2/5] c++: Set the locus of the function result decl
2022-11-15 23:52 ` Jason Merrill
@ 2022-11-17 8:56 ` Bernhard Reutner-Fischer
2022-11-17 14:53 ` Jason Merrill
0 siblings, 1 reply; 34+ messages in thread
From: Bernhard Reutner-Fischer @ 2022-11-17 8:56 UTC (permalink / raw)
To: Jason Merrill
Cc: rep.dot.nop, gcc-patches, Bernhard Reutner-Fischer, Nathan Sidwell
[-- Attachment #1: Type: text/plain, Size: 4491 bytes --]
On Tue, 15 Nov 2022 18:52:41 -0500
Jason Merrill <jason@redhat.com> wrote:
> On 11/12/22 13:45, Bernhard Reutner-Fischer wrote:
> > gcc/cp/ChangeLog:
> >
> > * decl.cc (start_function): Set the result decl source location to
> > the location of the typespec.
> >
> > ---
> > Bootstrapped and regtested on x86_86-unknown-linux with no regressions.
> > Ok for trunk?
> >
> > Cc: Nathan Sidwell <nathan@acm.org>
> > Cc: Jason Merrill <jason@redhat.com>
> > ---
> > gcc/cp/decl.cc | 15 ++++++++++++++-
> > 1 file changed, 14 insertions(+), 1 deletion(-)
> >
> > diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
> > index 6e98ea35a39..ed40815e645 100644
> > --- a/gcc/cp/decl.cc
> > +++ b/gcc/cp/decl.cc
> > @@ -17449,6 +17449,8 @@ start_function (cp_decl_specifier_seq *declspecs,
> > tree attrs)
> > {
> > tree decl1;
> > + tree result;
> > + bool ret;
>
> We now prefer to declare new variables as late as possible, usually when
> they are initialized.
Moved. Ok like attached? Bootstrapped and regtested fine.
> > decl1 = grokdeclarator (declarator, declspecs, FUNCDEF, 1, &attrs);
> > invoke_plugin_callbacks (PLUGIN_START_PARSE_FUNCTION, decl1);
> > @@ -17461,7 +17463,18 @@ 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);
> > + ret = start_preparsed_function (decl1, attrs, /*flags=*/SF_DEFAULT);
> > +
> > + /* decl1 might be ggc_freed here. */
> > + decl1 = current_function_decl;
> > +
> > + /* 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];
>
> One way to handle the template case would be for the code in
> start_preparsed_function that sets DECL_RESULT to check whether decl1 is
> a template instantiation, and in that case copy the location from the
> template's DECL_RESULT, i.e.
>
> DECL_RESULT (DECL_TEMPLATE_RESULT (DECL_TI_TEMPLATE (decl1)))
Well, that would probably work if something would set the location of
that template result decl properly, which nothing does out of the box.
diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
index ed7226b82f0..65d78c82a2d 100644
--- a/gcc/cp/decl.cc
+++ b/gcc/cp/decl.cc
@@ -17230,6 +17231,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_USE_TEMPLATE (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. */
(gdb) call inform(DECL_SOURCE_LOCATION (DECL_RESULT (decl1)), "decl1 result locus before")
../tmp4/return-narrow-2.cc:7:3: note: decl1 result locus before
7 | { return _M_finish != 0; }
| ^
(gdb) n
(gdb) call inform(DECL_SOURCE_LOCATION (DECL_RESULT (decl1)), "decl1 result locus from TI")
../tmp4/return-narrow-2.cc:7:3: note: decl1 result locus from TI
(gdb) p DECL_SOURCE_LOCATION (DECL_RESULT (decl1))
$1 = 267168
I'm leaving the template case alone for now, maybe i'm motivated later
on to again look at grokfndecl and/or grokmethod to fill in the proper
location. For starters i only need normal functions.
But many thanks for the hint on where the template stuff is, i thought
i would not need it at all but had hoped that there is a spot where
both declspec are at hand and something is "derived" from the templates.
>
> > + return ret;
> > }
> > \f
> > /* Returns true iff an EH_SPEC_BLOCK should be created in the body of
>
[-- Attachment #2: 0002-c-Set-the-locus-of-the-function-result-decl.patch --]
[-- Type: text/x-patch, Size: 1455 bytes --]
From 5595eb2d3056f6bca3d2b80bc8b2796d86da0ce8 Mon Sep 17 00:00:00 2001
From: Bernhard Reutner-Fischer <rep.dot.nop@gmail.com>
Date: Sun, 13 Nov 2022 00:45:40 +0100
Subject: [PATCH 2/5] c++: Set the locus of the function result decl
gcc/cp/ChangeLog:
* decl.cc (start_function): Set the result decl source location to
the location of the typespec.
---
gcc/cp/decl.cc | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
index 890cfcabd35..ed7226b82f0 100644
--- a/gcc/cp/decl.cc
+++ b/gcc/cp/decl.cc
@@ -17527,7 +17527,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
--
2.30.2
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 2/5] c++: Set the locus of the function result decl
2022-11-17 8:56 ` Bernhard Reutner-Fischer
@ 2022-11-17 14:53 ` Jason Merrill
2022-11-17 19:02 ` Bernhard Reutner-Fischer
0 siblings, 1 reply; 34+ messages in thread
From: Jason Merrill @ 2022-11-17 14:53 UTC (permalink / raw)
To: Bernhard Reutner-Fischer
Cc: gcc-patches, Bernhard Reutner-Fischer, Nathan Sidwell
On 11/17/22 03:56, Bernhard Reutner-Fischer wrote:
> On Tue, 15 Nov 2022 18:52:41 -0500
> Jason Merrill <jason@redhat.com> wrote:
>
>> On 11/12/22 13:45, Bernhard Reutner-Fischer wrote:
>>> gcc/cp/ChangeLog:
>>>
>>> * decl.cc (start_function): Set the result decl source location to
>>> the location of the typespec.
>>>
>>> ---
>>> Bootstrapped and regtested on x86_86-unknown-linux with no regressions.
>>> Ok for trunk?
>>>
>>> Cc: Nathan Sidwell <nathan@acm.org>
>>> Cc: Jason Merrill <jason@redhat.com>
>>> ---
>>> gcc/cp/decl.cc | 15 ++++++++++++++-
>>> 1 file changed, 14 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
>>> index 6e98ea35a39..ed40815e645 100644
>>> --- a/gcc/cp/decl.cc
>>> +++ b/gcc/cp/decl.cc
>>> @@ -17449,6 +17449,8 @@ start_function (cp_decl_specifier_seq *declspecs,
>>> tree attrs)
>>> {
>>> tree decl1;
>>> + tree result;
>>> + bool ret;
>>
>> We now prefer to declare new variables as late as possible, usually when
>> they are initialized.
>
> Moved. Ok like attached? Bootstrapped and regtested fine.
>
>>> decl1 = grokdeclarator (declarator, declspecs, FUNCDEF, 1, &attrs);
>>> invoke_plugin_callbacks (PLUGIN_START_PARSE_FUNCTION, decl1);
>>> @@ -17461,7 +17463,18 @@ 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);
>>> + ret = start_preparsed_function (decl1, attrs, /*flags=*/SF_DEFAULT);
>>> +
>>> + /* decl1 might be ggc_freed here. */
>>> + decl1 = current_function_decl;
>>> +
>>> + /* 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];
>>
>> One way to handle the template case would be for the code in
>> start_preparsed_function that sets DECL_RESULT to check whether decl1 is
>> a template instantiation, and in that case copy the location from the
>> template's DECL_RESULT, i.e.
>>
>> DECL_RESULT (DECL_TEMPLATE_RESULT (DECL_TI_TEMPLATE (decl1)))
>
> Well, that would probably work if something would set the location of
> that template result decl properly, which nothing does out of the box.
Hmm, it should get set by your patch, since templates go through
start_function like normal functions.
> diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
> index ed7226b82f0..65d78c82a2d 100644
> --- a/gcc/cp/decl.cc
> +++ b/gcc/cp/decl.cc
> @@ -17230,6 +17231,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_USE_TEMPLATE (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))))
This condition is true only for the template definition, for which you
haven't gotten to your start_function change yet.
Instead, you want to copy the location for instantiations, i.e. check
DECL_TEMPLATE_INSTANTIATION instead of !DECL_USE_TEMPLATE.
> + DECL_SOURCE_LOCATION (DECL_RESULT (decl1))
> + = DECL_SOURCE_LOCATION (
Open paren goes on the new line.
> + 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. */
>
> (gdb) call inform(DECL_SOURCE_LOCATION (DECL_RESULT (decl1)), "decl1 result locus before")
> ../tmp4/return-narrow-2.cc:7:3: note: decl1 result locus before
> 7 | { return _M_finish != 0; }
> | ^
> (gdb) n
> (gdb) call inform(DECL_SOURCE_LOCATION (DECL_RESULT (decl1)), "decl1 result locus from TI")
> ../tmp4/return-narrow-2.cc:7:3: note: decl1 result locus from TI
> (gdb) p DECL_SOURCE_LOCATION (DECL_RESULT (decl1))
> $1 = 267168
>
> I'm leaving the template case alone for now, maybe i'm motivated later
> on to again look at grokfndecl and/or grokmethod to fill in the proper
> location. For starters i only need normal functions.
> But many thanks for the hint on where the template stuff is, i thought
> i would not need it at all but had hoped that there is a spot where
> both declspec are at hand and something is "derived" from the templates.
>
>>
>>> + return ret;
>>> }
>>> \f
>>> /* Returns true iff an EH_SPEC_BLOCK should be created in the body of
>>
>
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 2/5] c++: Set the locus of the function result decl
2022-11-17 14:53 ` Jason Merrill
@ 2022-11-17 19:02 ` Bernhard Reutner-Fischer
2022-11-17 23:52 ` Jason Merrill
0 siblings, 1 reply; 34+ messages in thread
From: Bernhard Reutner-Fischer @ 2022-11-17 19:02 UTC (permalink / raw)
To: Jason Merrill
Cc: rep.dot.nop, gcc-patches, Bernhard Reutner-Fischer, Nathan Sidwell
On Thu, 17 Nov 2022 09:53:32 -0500
Jason Merrill <jason@redhat.com> wrote:
> On 11/17/22 03:56, Bernhard Reutner-Fischer wrote:
> > On Tue, 15 Nov 2022 18:52:41 -0500
> > Jason Merrill <jason@redhat.com> wrote:
> >
> >> On 11/12/22 13:45, Bernhard Reutner-Fischer wrote:
> >>> gcc/cp/ChangeLog:
> >>>
> >>> * decl.cc (start_function): Set the result decl source location to
> >>> the location of the typespec.
> >>>
> >>> ---
> >>> Bootstrapped and regtested on x86_86-unknown-linux with no regressions.
> >>> Ok for trunk?
> >>>
> >>> Cc: Nathan Sidwell <nathan@acm.org>
> >>> Cc: Jason Merrill <jason@redhat.com>
> >>> ---
> >>> gcc/cp/decl.cc | 15 ++++++++++++++-
> >>> 1 file changed, 14 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
> >>> index 6e98ea35a39..ed40815e645 100644
> >>> --- a/gcc/cp/decl.cc
> >>> +++ b/gcc/cp/decl.cc
> >>> @@ -17449,6 +17449,8 @@ start_function (cp_decl_specifier_seq *declspecs,
> >>> tree attrs)
> >>> {
> >>> tree decl1;
> >>> + tree result;
> >>> + bool ret;
> >>
> >> We now prefer to declare new variables as late as possible, usually when
> >> they are initialized.
> >
> > Moved. Ok like attached? Bootstrapped and regtested fine.
> >
> >>> decl1 = grokdeclarator (declarator, declspecs, FUNCDEF, 1, &attrs);
> >>> invoke_plugin_callbacks (PLUGIN_START_PARSE_FUNCTION, decl1);
> >>> @@ -17461,7 +17463,18 @@ 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);
> >>> + ret = start_preparsed_function (decl1, attrs, /*flags=*/SF_DEFAULT);
> >>> +
> >>> + /* decl1 might be ggc_freed here. */
> >>> + decl1 = current_function_decl;
> >>> +
> >>> + /* 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];
> >>
> >> One way to handle the template case would be for the code in
> >> start_preparsed_function that sets DECL_RESULT to check whether decl1 is
> >> a template instantiation, and in that case copy the location from the
> >> template's DECL_RESULT, i.e.
> >>
> >> DECL_RESULT (DECL_TEMPLATE_RESULT (DECL_TI_TEMPLATE (decl1)))
> >
> > Well, that would probably work if something would set the location of
> > that template result decl properly, which nothing does out of the box.
>
> Hmm, it should get set by your patch, since templates go through
> start_function like normal functions.
>
> > diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
> > index ed7226b82f0..65d78c82a2d 100644
> > --- a/gcc/cp/decl.cc
> > +++ b/gcc/cp/decl.cc
> > @@ -17230,6 +17231,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_USE_TEMPLATE (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))))
>
> This condition is true only for the template definition, for which you
> haven't gotten to your start_function change yet.
>
> 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.
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.
Is the hunk for normal functions OK for trunk?
thanks,
>
> > + DECL_SOURCE_LOCATION (DECL_RESULT (decl1))
> > + = DECL_SOURCE_LOCATION (
>
> Open paren goes on the new line.
>
> > + 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. */
> >
> > (gdb) call inform(DECL_SOURCE_LOCATION (DECL_RESULT (decl1)), "decl1 result locus before")
> > ../tmp4/return-narrow-2.cc:7:3: note: decl1 result locus before
> > 7 | { return _M_finish != 0; }
> > | ^
> > (gdb) n
> > (gdb) call inform(DECL_SOURCE_LOCATION (DECL_RESULT (decl1)), "decl1 result locus from TI")
> > ../tmp4/return-narrow-2.cc:7:3: note: decl1 result locus from TI
> > (gdb) p DECL_SOURCE_LOCATION (DECL_RESULT (decl1))
> > $1 = 267168
> >
> > I'm leaving the template case alone for now, maybe i'm motivated later
> > on to again look at grokfndecl and/or grokmethod to fill in the proper
> > location. For starters i only need normal functions.
> > But many thanks for the hint on where the template stuff is, i thought
> > i would not need it at all but had hoped that there is a spot where
> > both declspec are at hand and something is "derived" from the templates.
> >
> >>
> >>> + return ret;
> >>> }
> >>> \f
> >>> /* Returns true iff an EH_SPEC_BLOCK should be created in the body of
> >>
> >
>
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 2/5] c++: Set the locus of the function result decl
2022-11-17 19:02 ` Bernhard Reutner-Fischer
@ 2022-11-17 23:52 ` Jason Merrill
2022-11-18 10:49 ` Bernhard Reutner-Fischer
0 siblings, 1 reply; 34+ messages in thread
From: Jason Merrill @ 2022-11-17 23:52 UTC (permalink / raw)
To: Bernhard Reutner-Fischer
Cc: gcc-patches, Bernhard Reutner-Fischer, Nathan Sidwell
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:
>
>> On 11/17/22 03:56, Bernhard Reutner-Fischer wrote:
>>> On Tue, 15 Nov 2022 18:52:41 -0500
>>> Jason Merrill <jason@redhat.com> wrote:
>>>
>>>> On 11/12/22 13:45, Bernhard Reutner-Fischer wrote:
>>>>> gcc/cp/ChangeLog:
>>>>>
>>>>> * decl.cc (start_function): Set the result decl source location to
>>>>> the location of the typespec.
>>>>>
>>>>> ---
>>>>> Bootstrapped and regtested on x86_86-unknown-linux with no regressions.
>>>>> Ok for trunk?
>>>>>
>>>>> Cc: Nathan Sidwell <nathan@acm.org>
>>>>> Cc: Jason Merrill <jason@redhat.com>
>>>>> ---
>>>>> gcc/cp/decl.cc | 15 ++++++++++++++-
>>>>> 1 file changed, 14 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
>>>>> index 6e98ea35a39..ed40815e645 100644
>>>>> --- a/gcc/cp/decl.cc
>>>>> +++ b/gcc/cp/decl.cc
>>>>> @@ -17449,6 +17449,8 @@ start_function (cp_decl_specifier_seq *declspecs,
>>>>> tree attrs)
>>>>> {
>>>>> tree decl1;
>>>>> + tree result;
>>>>> + bool ret;
>>>>
>>>> We now prefer to declare new variables as late as possible, usually when
>>>> they are initialized.
>>>
>>> Moved. Ok like attached? Bootstrapped and regtested fine.
>>>
>>>>> decl1 = grokdeclarator (declarator, declspecs, FUNCDEF, 1, &attrs);
>>>>> invoke_plugin_callbacks (PLUGIN_START_PARSE_FUNCTION, decl1);
>>>>> @@ -17461,7 +17463,18 @@ 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);
>>>>> + ret = start_preparsed_function (decl1, attrs, /*flags=*/SF_DEFAULT);
>>>>> +
>>>>> + /* decl1 might be ggc_freed here. */
>>>>> + decl1 = current_function_decl;
>>>>> +
>>>>> + /* 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];
>>>>
>>>> One way to handle the template case would be for the code in
>>>> start_preparsed_function that sets DECL_RESULT to check whether decl1 is
>>>> a template instantiation, and in that case copy the location from the
>>>> template's DECL_RESULT, i.e.
>>>>
>>>> DECL_RESULT (DECL_TEMPLATE_RESULT (DECL_TI_TEMPLATE (decl1)))
>>>
>>> Well, that would probably work if something would set the location of
>>> that template result decl properly, which nothing does out of the box.
>>
>> Hmm, it should get set by your patch, since templates go through
>> start_function like normal functions.
>>
>>> diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
>>> index ed7226b82f0..65d78c82a2d 100644
>>> --- a/gcc/cp/decl.cc
>>> +++ b/gcc/cp/decl.cc
>>> @@ -17230,6 +17231,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_USE_TEMPLATE (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))))
>>
>> This condition is true only for the template definition, for which you
>> haven't gotten to your start_function change yet.
>>
>> 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}
> 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:" }
> thanks,
>
>>
>>> + DECL_SOURCE_LOCATION (DECL_RESULT (decl1))
>>> + = DECL_SOURCE_LOCATION (
>>
>> Open paren goes on the new line.
>>
>>> + 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. */
>>>
>>> (gdb) call inform(DECL_SOURCE_LOCATION (DECL_RESULT (decl1)), "decl1 result locus before")
>>> ../tmp4/return-narrow-2.cc:7:3: note: decl1 result locus before
>>> 7 | { return _M_finish != 0; }
>>> | ^
>>> (gdb) n
>>> (gdb) call inform(DECL_SOURCE_LOCATION (DECL_RESULT (decl1)), "decl1 result locus from TI")
>>> ../tmp4/return-narrow-2.cc:7:3: note: decl1 result locus from TI
>>> (gdb) p DECL_SOURCE_LOCATION (DECL_RESULT (decl1))
>>> $1 = 267168
>>>
>>> I'm leaving the template case alone for now, maybe i'm motivated later
>>> on to again look at grokfndecl and/or grokmethod to fill in the proper
>>> location. For starters i only need normal functions.
>>> But many thanks for the hint on where the template stuff is, i thought
>>> i would not need it at all but had hoped that there is a spot where
>>> both declspec are at hand and something is "derived" from the templates.
>>>
>>>>
>>>>> + return ret;
>>>>> }
>>>>> \f
>>>>> /* Returns true iff an EH_SPEC_BLOCK should be created in the body of
>>>>
>>>
>>
>
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 2/5] c++: Set the locus of the function result decl
2022-11-17 23:52 ` Jason Merrill
@ 2022-11-18 10:49 ` Bernhard Reutner-Fischer
2022-11-18 16:06 ` Jason Merrill
0 siblings, 1 reply; 34+ messages in thread
From: Bernhard Reutner-Fischer @ 2022-11-18 10:49 UTC (permalink / raw)
To: Jason Merrill
Cc: rep.dot.nop, gcc-patches, Bernhard Reutner-Fischer, Nathan Sidwell
[-- 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 >;
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 2/5] c++: Set the locus of the function result decl
2022-11-18 10:49 ` Bernhard Reutner-Fischer
@ 2022-11-18 16:06 ` Jason Merrill
2022-11-18 18:26 ` Bernhard Reutner-Fischer
2022-11-19 9:56 ` Bernhard Reutner-Fischer
0 siblings, 2 replies; 34+ messages in thread
From: Jason Merrill @ 2022-11-18 16:06 UTC (permalink / raw)
To: Bernhard Reutner-Fischer
Cc: gcc-patches, Bernhard Reutner-Fischer, Nathan Sidwell
On 11/18/22 05:49, Bernhard Reutner-Fischer wrote:
> 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.
Ah, so the problem is deferred parsing of methods, rather than
templates. Building the DECL_RESULT sooner does seem like the right
approach to handling that, whether that's in grokfndecl or grokmethod.
> 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?
Good point. Do any of your later patches add such a warning?
> 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,
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 2/5] c++: Set the locus of the function result decl
2022-11-18 16:06 ` Jason Merrill
@ 2022-11-18 18:26 ` Bernhard Reutner-Fischer
2022-11-19 9:56 ` Bernhard Reutner-Fischer
1 sibling, 0 replies; 34+ messages in thread
From: Bernhard Reutner-Fischer @ 2022-11-18 18:26 UTC (permalink / raw)
To: Jason Merrill
Cc: rep.dot.nop, gcc-patches, Bernhard Reutner-Fischer, Nathan Sidwell
On Fri, 18 Nov 2022 11:06:29 -0500
Jason Merrill <jason@redhat.com> wrote:
> Ah, so the problem is deferred parsing of methods, rather than
> templates. Building the DECL_RESULT sooner does seem like the right
> approach to handling that, whether that's in grokfndecl or grokmethod.
> >> 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.
Please do, i'd be glad if you could take care of these locations.
It icks me that they are wrong, and be it just for the sake of QOI :)
> >>> 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?
>
> Good point. Do any of your later patches add such a warning?
I didn't mean to have that -Wtype-demotion applied in it's current
form, or at all, so no. I was curious if anybody liked the idea of
pointing out such code though. I've had no feedback but everybody is or
was busy with end of stage3 and real work, so that's expected. The only
real purpose i had for it was to find places in the Fortran FE that
could use narrower types, bools for the most part.
IMHO it would be a nice thing to have, but then, embedded software
usually is cautious to use sensible types in the first place and the
rest doesn't really care anyway, supposedly.
Maybe it would have made more sense to just do an IPA pass that does the
demotion silently where it's feasable.
As to the test, i don't think these locations in the c++ FE are changed
all that often, so chances are rather low that they would be broken
once in.
So, short of trying to use the result decl locus for any existing
-Wreturn-type, -Waggregate-return, -Wno-return-local-addr,
-Wsuggest-attribute=[pure|const|noreturn|format|malloc] or another
existing warning that would be concerned, we could, as said, have a
plugin with fix-it hints and ideally -fdiagnostics-generate-patch to
test these bits. Patch generation has the advantage that it will ICE
more often than not if asked to generate patches for locations that
have a negative relative start (think: memcpy(...,..., -7)), which you
can get easily if the locations are off IMHO.
> > Maybe a g++.dg/plugin/result_decl_plugin.c then.
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 2/5] c++: Set the locus of the function result decl
2022-11-18 16:06 ` Jason Merrill
2022-11-18 18:26 ` Bernhard Reutner-Fischer
@ 2022-11-19 9:56 ` Bernhard Reutner-Fischer
2022-11-20 17:06 ` Bernhard Reutner-Fischer
1 sibling, 1 reply; 34+ messages in thread
From: Bernhard Reutner-Fischer @ 2022-11-19 9:56 UTC (permalink / raw)
To: gcc-patches; +Cc: Bernhard Reutner-Fischer, Bernhard Reutner-Fischer, jason
Hi Jason!
Possible test.
An existing test might be to equip the existing warning for
bool unsigned double meh(void) {return 0;}
with a fix-it hint instead of the brief
error: two or more data types in declaration of ‘meh’.
Likewise for
bool unsigned meh(void) {return 0;}
error: ‘unsigned’ specified with ‘bool’
so we wouldn't need a plugin, and it might even be useful? ;)
cheers,
* g++.dg/plugin/plugin.exp: Add new test.
* g++.dg/plugin/result-decl-plugin-test-1.C: New test.
* g++.dg/plugin/result-decl-plugin-test-2.C: New test.
* g++.dg/plugin/result_decl_plugin.C: New test.
---
gcc/testsuite/g++.dg/plugin/plugin.exp | 3 +
.../g++.dg/plugin/result-decl-plugin-test-1.C | 28 +++++++++
.../g++.dg/plugin/result-decl-plugin-test-2.C | 61 +++++++++++++++++++
.../g++.dg/plugin/result_decl_plugin.C | 57 +++++++++++++++++
4 files changed, 149 insertions(+)
create mode 100644 gcc/testsuite/g++.dg/plugin/result-decl-plugin-test-1.C
create mode 100644 gcc/testsuite/g++.dg/plugin/result-decl-plugin-test-2.C
create mode 100644 gcc/testsuite/g++.dg/plugin/result_decl_plugin.C
diff --git a/gcc/testsuite/g++.dg/plugin/plugin.exp b/gcc/testsuite/g++.dg/plugin/plugin.exp
index b5fb42fa77a..f2b526b4704 100644
--- a/gcc/testsuite/g++.dg/plugin/plugin.exp
+++ b/gcc/testsuite/g++.dg/plugin/plugin.exp
@@ -80,6 +80,9 @@ set plugin_test_list [list \
show-template-tree-color-labels.C \
show-template-tree-color-no-elide-type.C } \
{ comment_plugin.c comments-1.C } \
+ { result_decl_plugin.C \
+ result-decl-plugin-test-1.C \
+ result-decl-plugin-test-2.C } \
]
foreach plugin_test $plugin_test_list {
diff --git a/gcc/testsuite/g++.dg/plugin/result-decl-plugin-test-1.C b/gcc/testsuite/g++.dg/plugin/result-decl-plugin-test-1.C
new file mode 100644
index 00000000000..bd323181d70
--- /dev/null
+++ b/gcc/testsuite/g++.dg/plugin/result-decl-plugin-test-1.C
@@ -0,0 +1,28 @@
+/* Verify that class member functions result decl have the correct location. */
+// { dg-options "-fdiagnostics-generate-patch" }
+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 >;
+#if 0
+{ dg-begin-multiline-output "" }
+@@ -5,7 +5,7 @@ template < typename > struct __mini_vect
+ {
+ int _M_finish;
+ const
+- unsigned long
++ bool
+ __attribute__((deprecated))
+ _M_space_left()
+ { return _M_finish != 0; }
+
+{ dg-end-multiline-output "" }
+#endif
diff --git a/gcc/testsuite/g++.dg/plugin/result-decl-plugin-test-2.C b/gcc/testsuite/g++.dg/plugin/result-decl-plugin-test-2.C
new file mode 100644
index 00000000000..385a7ef482f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/plugin/result-decl-plugin-test-2.C
@@ -0,0 +1,61 @@
+/* Verify that template functions result decl have the correct location. */
+// { dg-options "-fdiagnostics-generate-patch" }
+template <class T>
+int
+f()
+{
+ return 42;
+}
+int main()
+{
+ f<int>();
+}
+unsigned long long huh(void)
+{
+ return 1ULL;
+}
+#if 0
+{ dg-begin-multiline-output "" }
+g++.dg/plugin/result-decl-plugin-test-2.C:4:1: warning: Function ‘f’ result location
+ 4 | int
+ | ^~~
+ | bool
+g++.dg/plugin/result-decl-plugin-test-2.C:9:1: warning: Function ‘main’ result location
+ 9 | int main()
+ | ^~~
+ | bool
+g++.dg/plugin/result-decl-plugin-test-2.C:13:28: warning: Function ‘huh’ result location
+ 13 | unsigned long long huh(void)
+ | ^
+ | bool
+g++.dg/plugin/result-decl-plugin-test-2.C: In instantiation of ‘int f() [with T = int]’:
+g++.dg/plugin/result-decl-plugin-test-2.C:11:10: required from here
+g++.dg/plugin/result-decl-plugin-test-2.C:4:1: warning: Function ‘f’ result location
+ 4 | int
+ | ^~~
+ | bool
+--- g++.dg/plugin/result-decl-plugin-test-2.C
++++ g++.dg/plugin/result-decl-plugin-test-2.C
+@@ -1,16 +1,16 @@
+ /* Verify that template functions result decl have the correct location. */
+ // { dg-options "-fdiagnostics-generate-patch" }
+ template <class T>
+-int
++bool
+ f()
+ {
+ return 42;
+ }
+-int main()
++bool main()
+ {
+ f<int>();
+ }
+-unsigned long long huh(void)
++unsigned long long huh(voidbool
+ {
+ return 1ULL;
+ }
+{ dg-end-multiline-output "" }
+#endif
+// Note: f() should not +bbool with an off-by-one for the start 'b' !
diff --git a/gcc/testsuite/g++.dg/plugin/result_decl_plugin.C b/gcc/testsuite/g++.dg/plugin/result_decl_plugin.C
new file mode 100644
index 00000000000..40f54a6acfe
--- /dev/null
+++ b/gcc/testsuite/g++.dg/plugin/result_decl_plugin.C
@@ -0,0 +1,57 @@
+/* A plugin example that points at the location of function decl result decl */
+/* This file is part of GCC */
+/* { dg-options "-O" } */
+#include "gcc-plugin.h"
+#include "config.h"
+#include "system.h"
+#include "coretypes.h"
+#include "tree.h"
+#include "tree-pass.h"
+#include "intl.h"
+#include "diagnostic.h"
+#include "gcc-rich-location.h"
+#include "langhooks.h"
+#include "plugin-version.h"
+
+static struct plugin_info _this_info = {
+ .version = "1",
+ .help = "None",
+};
+/* Callback function to invoke after GCC finishes a declaration. */
+
+static void plugin_finish_decl (void *event_data, void *data)
+{
+ tree decl = (tree) event_data;
+ if (!decl || TREE_CODE (decl) != FUNCTION_DECL)
+ return;
+
+ tree logical_1_t = boolean_type_node;
+ tree logical_1_n = DECL_NAME (TYPE_NAME (logical_1_t));
+ location_t result_loc = DECL_SOURCE_LOCATION (DECL_RESULT (decl));
+ gcc_rich_location richloc (result_loc);
+ richloc.add_fixit_replace (result_loc, IDENTIFIER_POINTER (logical_1_n));
+
+ warning_at (&richloc, 0, G_("Function %qs result location"),
+ IDENTIFIER_POINTER (DECL_NAME (decl)));
+}
+
+int
+plugin_init (struct plugin_name_args *plugin_info,
+ struct plugin_gcc_version *version)
+{
+ if (!plugin_default_version_check (version, &gcc_version))
+ {
+ error(G_("incompatible gcc/plugin versions"));
+ return 1;
+ }
+
+ const char *plugin_name = plugin_info->base_name;
+
+ register_callback(plugin_name, PLUGIN_INFO, NULL, &_this_info);
+ register_callback (plugin_name, PLUGIN_FINISH_PARSE_FUNCTION,
+ plugin_finish_decl, NULL);
+ return 0;
+}
+
+/* Announce that we are GPL. */
+int plugin_is_GPL_compatible;
--
2.38.1
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 2/5] c++: Set the locus of the function result decl
2022-11-19 9:56 ` Bernhard Reutner-Fischer
@ 2022-11-20 17:06 ` Bernhard Reutner-Fischer
2022-11-22 20:25 ` Jason Merrill
0 siblings, 1 reply; 34+ messages in thread
From: Bernhard Reutner-Fischer @ 2022-11-20 17:06 UTC (permalink / raw)
To: gcc-patches; +Cc: Bernhard Reutner-Fischer, Bernhard Reutner-Fischer, jason
Hi Jason!
The "meh" of result-decl-plugin-test-2.C should likely be omitted,
grokdeclarator would need some changes to add richloc hints and we would not
be able to make a reliable guess what to remove precisely.
C.f. /* Check all other uses of type modifiers. */
Furthermore it is unrelated to DECL_RESULT so not of direct interest
here. The other tests in test-2.C, f() and huh() should work though.
I don't know if it's acceptable to change ipa-pure-const to make the
missing noreturn warning more precise and emit a fixit-hint. At least it
would be a real test for the DECL_RESULT and would spare us the plugin.
HTH,
gcc/cp/ChangeLog:
* decl.cc (start_preparsed_function): Set the result decl source
location to the location of the typespec.
(start_function): Likewise.
gcc/ChangeLog:
* ipa-pure-const.cc (suggest_attribute): Add fixit-hint for the
noreturn attribute.
gcc/testsuite/ChangeLog:
* c-c++-common/pr68833-1.c: Adjust noreturn warning line number.
* gcc.dg/noreturn-1.c: Likewise.
* g++.dg/plugin/plugin.exp: Add new plugin test.
* g++.dg/other/resultdecl-1.C: New test.
* g++.dg/plugin/result-decl-plugin-test-1.C: New test.
* g++.dg/plugin/result-decl-plugin-test-2.C: New test.
* g++.dg/plugin/result_decl_plugin.C: New test.
---
gcc/cp/decl.cc | 26 +++++++-
gcc/ipa-pure-const.cc | 14 ++++-
gcc/testsuite/c-c++-common/pr68833-1.c | 2 +-
gcc/testsuite/g++.dg/other/resultdecl-1.C | 32 ++++++++++
gcc/testsuite/g++.dg/plugin/plugin.exp | 3 +
.../g++.dg/plugin/result-decl-plugin-test-1.C | 31 ++++++++++
.../g++.dg/plugin/result-decl-plugin-test-2.C | 59 +++++++++++++++++++
.../g++.dg/plugin/result_decl_plugin.C | 53 +++++++++++++++++
gcc/testsuite/gcc.dg/noreturn-1.c | 2 +-
9 files changed, 218 insertions(+), 4 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/other/resultdecl-1.C
create mode 100644 gcc/testsuite/g++.dg/plugin/result-decl-plugin-test-1.C
create mode 100644 gcc/testsuite/g++.dg/plugin/result-decl-plugin-test-2.C
create mode 100644 gcc/testsuite/g++.dg/plugin/result_decl_plugin.C
diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
index d28889ed865..0c053b6d19f 100644
--- a/gcc/cp/decl.cc
+++ b/gcc/cp/decl.cc
@@ -17235,6 +17235,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 +17543,20 @@ 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 (ret
+ && 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
diff --git a/gcc/ipa-pure-const.cc b/gcc/ipa-pure-const.cc
index 572a6da274f..1c80034f38d 100644
--- a/gcc/ipa-pure-const.cc
+++ b/gcc/ipa-pure-const.cc
@@ -63,6 +63,7 @@ along with GCC; see the file COPYING3. If not see
#include "ipa-fnsummary.h"
#include "symtab-thunks.h"
#include "dbgcnt.h"
+#include "gcc-rich-location.h"
/* Lattice values for const and pure functions. Everything starts out
being const, then may drop to pure and then neither depending on
@@ -212,7 +213,18 @@ suggest_attribute (int option, tree decl, bool known_finite,
if (warned_about->contains (decl))
return warned_about;
warned_about->add (decl);
- warning_at (DECL_SOURCE_LOCATION (decl),
+
+ gcc_rich_location richloc (option == OPT_Wsuggest_attribute_noreturn
+ ? DECL_SOURCE_LOCATION (DECL_RESULT (decl))
+ : DECL_SOURCE_LOCATION (decl));
+ if (option == OPT_Wsuggest_attribute_noreturn)
+ {
+ richloc.add_fixit_replace (IDENTIFIER_POINTER (DECL_NAME (TYPE_NAME (
+ void_type_node))));
+ richloc.add_fixit_insert_before (DECL_SOURCE_LOCATION (decl),
+ " __attribute__((__noreturn__)) ");
+ }
+ warning_at (&richloc,
option,
known_finite
? G_("function might be candidate for attribute %qs")
diff --git a/gcc/testsuite/c-c++-common/pr68833-1.c b/gcc/testsuite/c-c++-common/pr68833-1.c
index a6aefad5c98..b27b783d61a 100644
--- a/gcc/testsuite/c-c++-common/pr68833-1.c
+++ b/gcc/testsuite/c-c++-common/pr68833-1.c
@@ -15,7 +15,7 @@ f1 (const char *fmt)
extern void f2 (void);
void
-f2 (void) /* { dg-error "candidate for attribute 'noreturn'" "detect noreturn candidate" } */
+f2 (void) /* { dg-error "candidate for attribute 'noreturn'" "detect noreturn candidate" { target *-*-* } .-1 } */
{
__builtin_exit (0);
}
diff --git a/gcc/testsuite/g++.dg/other/resultdecl-1.C b/gcc/testsuite/g++.dg/other/resultdecl-1.C
new file mode 100644
index 00000000000..e3951c339a9
--- /dev/null
+++ b/gcc/testsuite/g++.dg/other/resultdecl-1.C
@@ -0,0 +1,32 @@
+/* Verify correct result decl location. */
+// { dg-options "-fdiagnostics-generate-patch -O1 -fipa-pure-const -Wsuggest-attribute=noreturn" }
+//
+// foo4: rem int, add: void attribute((noreturn))
+
+
+
+int
+foo4(void)
+{ while (1); }
+
+
+#if 0
+// { dg-warning "function might be candidate for attribute .noreturn." "" { target *-*-* } .-6 } // the fnname is at line .-5 though
+// prune the diff path..
+/* { dg-regexp "\\-\\-\\- .*" } */
+/* { dg-regexp "\\+\\+\\+ .*" } */
+{ dg-begin-multiline-output "" }
+@@ -5,8 +5,8 @@
+
+
+
+-int
+-foo4(void)
++void
++ __attribute__((__noreturn__)) foo4(void)
+ { while (1); }
+
+
+{ dg-end-multiline-output "" }
+#endif
+
diff --git a/gcc/testsuite/g++.dg/plugin/plugin.exp b/gcc/testsuite/g++.dg/plugin/plugin.exp
index b5fb42fa77a..21d7ef0313e 100644
--- a/gcc/testsuite/g++.dg/plugin/plugin.exp
+++ b/gcc/testsuite/g++.dg/plugin/plugin.exp
@@ -80,6 +80,9 @@ set plugin_test_list [list \
show-template-tree-color-labels.C \
show-template-tree-color-no-elide-type.C } \
{ comment_plugin.c comments-1.C } \
+ { result_decl_plugin.C \
+ result-decl-plugin-test-1.C \
+ result-decl-plugin-test-2.C } \
]
foreach plugin_test $plugin_test_list {
diff --git a/gcc/testsuite/g++.dg/plugin/result-decl-plugin-test-1.C b/gcc/testsuite/g++.dg/plugin/result-decl-plugin-test-1.C
new file mode 100644
index 00000000000..1bbdc4c8c44
--- /dev/null
+++ b/gcc/testsuite/g++.dg/plugin/result-decl-plugin-test-1.C
@@ -0,0 +1,31 @@
+/* Verify that class member functions result decl have the correct location. */
+// { dg-options "-fdiagnostics-generate-patch" }
+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 >;
+#if 0
+// { dg-warning "Function ._M_space_left. result location" "" { target *-*-* } .-5 }
+// prune the diff path..
+/* { dg-regexp "\\-\\-\\- .*" } */
+/* { dg-regexp "\\+\\+\\+ .*" } */
+{ dg-begin-multiline-output "" }
+@@ -8,7 +8,7 @@ template < typename > struct __mini_vect
+ {
+ int _M_finish;
+ const
+- unsigned long
++ bool
+ __attribute__((deprecated))
+ _M_space_left()
+ { return _M_finish != 0; }
+{ dg-end-multiline-output "" }
+#endif
diff --git a/gcc/testsuite/g++.dg/plugin/result-decl-plugin-test-2.C b/gcc/testsuite/g++.dg/plugin/result-decl-plugin-test-2.C
new file mode 100644
index 00000000000..c67388195a1
--- /dev/null
+++ b/gcc/testsuite/g++.dg/plugin/result-decl-plugin-test-2.C
@@ -0,0 +1,59 @@
+/* Verify that template functions result decl have the correct location. */
+// { dg-options "-fdiagnostics-generate-patch" }
+
+
+
+template <class T>
+int
+f()
+{
+ return 42;
+}
+int main()
+{
+ f<int>();
+}
+unsigned long long huh(void)
+{
+ return 1ULL;
+}
+bool unsigned meh(void) {return 0;}
+
+
+
+
+#if 0
+// { dg-warning "Function .f. result location" "" { target *-*-* } .-19 }
+// { dg-warning "Function .huh. result location" "" { target *-*-* } .-11 }
+// { dg-warning "Function .meh. result location" "" { target *-*-* } .-8 }
+// prune the diff path..
+/* { dg-regexp "\\-\\-\\- .*" } */
+/* { dg-regexp "\\+\\+\\+ .*" } */
+// { dg-prune ".unsigned. specified with .bool." }
+{ dg-begin-multiline-output "" }
+@@ -4,7 +4,7 @@
+
+
+ template <class T>
+-int
++bool
+ f()
+ {
+ return 42;
+@@ -13,11 +13,11 @@
+ {
+ f<int>();
+ }
+-unsigned long long huh(void)
++bool huh(void)
+ {
+ return 1ULL;
+ }
+-bool unsigned meh(void) {return 0;}
++bool meh(void) {return 0;}
+
+
+
+{ dg-end-multiline-output "" }
+#endif
+// Note: f() should NOT +bbool but just +bool
diff --git a/gcc/testsuite/g++.dg/plugin/result_decl_plugin.C b/gcc/testsuite/g++.dg/plugin/result_decl_plugin.C
new file mode 100644
index 00000000000..9ab408e8a65
--- /dev/null
+++ b/gcc/testsuite/g++.dg/plugin/result_decl_plugin.C
@@ -0,0 +1,53 @@
+/* A plugin example that points at the location of function result decl */
+/* This file is part of GCC */
+/* { dg-options "-O" } */
+#include "gcc-plugin.h"
+#include "config.h"
+#include "system.h"
+#include "coretypes.h"
+#include "tree.h"
+#include "intl.h"
+#include "diagnostic.h"
+#include "gcc-rich-location.h"
+#include "langhooks.h"
+#include "plugin-version.h"
+
+/* Callback function to invoke after GCC finishes a function definition. */
+
+static void plugin_finish_decl (void *event_data, void *data)
+{
+ tree decl = (tree) event_data;
+ if (!decl || TREE_CODE (decl) != FUNCTION_DECL)
+ return;
+ if (MAIN_NAME_P (DECL_NAME (decl)))
+ return;
+
+ tree logical_1_t = boolean_type_node;
+ tree logical_1_n = DECL_NAME (TYPE_NAME (logical_1_t));
+ location_t result_loc = DECL_SOURCE_LOCATION (DECL_RESULT (decl));
+ gcc_rich_location richloc (result_loc);
+ richloc.add_fixit_replace (result_loc, IDENTIFIER_POINTER (logical_1_n));
+
+ warning_at (&richloc, 0, G_("Function %qs result location"),
+ IDENTIFIER_POINTER (DECL_NAME (decl)));
+}
+
+int
+plugin_init (struct plugin_name_args *plugin_info,
+ struct plugin_gcc_version *version)
+{
+ if (!plugin_default_version_check (version, &gcc_version))
+ {
+ error(G_("incompatible gcc/plugin versions"));
+ return 1;
+ }
+
+ const char *plugin_name = plugin_info->base_name;
+
+ register_callback (plugin_name, PLUGIN_FINISH_PARSE_FUNCTION,
+ plugin_finish_decl, NULL);
+ return 0;
+}
+
+/* Announce that we are GPL. */
+int plugin_is_GPL_compatible;
diff --git a/gcc/testsuite/gcc.dg/noreturn-1.c b/gcc/testsuite/gcc.dg/noreturn-1.c
index cdbfb8dd667..a713ee924fc 100644
--- a/gcc/testsuite/gcc.dg/noreturn-1.c
+++ b/gcc/testsuite/gcc.dg/noreturn-1.c
@@ -25,7 +25,7 @@ foo3(void)
extern void foo4(void);
void
-foo4(void) /* { dg-warning "candidate for attribute 'noreturn'" "detect noreturn candidate" } */
+foo4(void) /* { dg-warning "candidate for attribute 'noreturn'" "detect noreturn candidate" { target *-*-* } .-1 } */
{
exit(0);
}
--
2.38.1
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 2/5] c++: Set the locus of the function result decl
2022-11-20 17:06 ` Bernhard Reutner-Fischer
@ 2022-11-22 20:25 ` Jason Merrill
2022-11-23 15:28 ` Jason Merrill
0 siblings, 1 reply; 34+ messages in thread
From: Jason Merrill @ 2022-11-22 20:25 UTC (permalink / raw)
To: Bernhard Reutner-Fischer, gcc-patches; +Cc: Bernhard Reutner-Fischer
[-- Attachment #1: Type: text/plain, Size: 921 bytes --]
On 11/20/22 12:06, Bernhard Reutner-Fischer wrote:
> Hi Jason!
>
> The "meh" of result-decl-plugin-test-2.C should likely be omitted,
> grokdeclarator would need some changes to add richloc hints and we would not
> be able to make a reliable guess what to remove precisely.
> C.f. /* Check all other uses of type modifiers. */
> Furthermore it is unrelated to DECL_RESULT so not of direct interest
> here. The other tests in test-2.C, f() and huh() should work though.
>
> I don't know if it's acceptable to change ipa-pure-const to make the
> missing noreturn warning more precise and emit a fixit-hint. At least it
> would be a real test for the DECL_RESULT and would spare us the plugin.
The main problem I see with that change is that the syntax of the fixit
might be wrong for non-C-family front-ends.
Here's a version of the patch that fixes template/method handling, and
adjusts -Waggregate-return as well:
[-- Attachment #2: 0001-c-Set-the-locus-of-the-function-result-decl.patch --]
[-- Type: text/x-patch, Size: 7595 bytes --]
From 5075d2ac12f655f8f83f6f3be27e2c1141e1ce99 Mon Sep 17 00:00:00 2001
From: Bernhard Reutner-Fischer <rep.dot.nop@gmail.com>
Date: Sun, 20 Nov 2022 18:06:04 +0100
Subject: [PATCH] c++: Set the locus of the function result decl
To: gcc-patches@gcc.gnu.org
gcc/cp/ChangeLog:
* decl.cc (grokdeclarator): Build RESULT_DECL.
(start_preparsed_function): Copy location from template.
gcc/ChangeLog:
* function.cc (init_function_start): Use DECL_RESULT location
for -Waggregate-return warning.
* ipa-pure-const.cc (suggest_attribute): Add fixit-hint for the
noreturn attribute.
gcc/testsuite/ChangeLog:
* c-c++-common/pr68833-1.c: Adjust noreturn warning line number.
* gcc.dg/noreturn-1.c: Likewise.
* g++.dg/diagnostic/return-type-loc1.C: New test.
* g++.dg/other/resultdecl-1.C: New test.
Co-authored-by: Jason Merrill <jason@redhat.com>
---
gcc/cp/decl.cc | 26 +++++++++++++--
gcc/function.cc | 3 +-
gcc/ipa-pure-const.cc | 14 +++++++-
gcc/testsuite/c-c++-common/pr68833-1.c | 2 +-
.../g++.dg/diagnostic/return-type-loc1.C | 20 ++++++++++++
gcc/testsuite/g++.dg/other/resultdecl-1.C | 32 +++++++++++++++++++
gcc/testsuite/gcc.dg/noreturn-1.c | 2 +-
7 files changed, 93 insertions(+), 6 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/diagnostic/return-type-loc1.C
create mode 100644 gcc/testsuite/g++.dg/other/resultdecl-1.C
diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
index 544efdc9914..2c5cd930e0a 100644
--- a/gcc/cp/decl.cc
+++ b/gcc/cp/decl.cc
@@ -14774,6 +14774,18 @@ grokdeclarator (const cp_declarator *declarator,
else if (constinit_p)
DECL_DECLARED_CONSTINIT_P (decl) = true;
}
+ else if (TREE_CODE (decl) == FUNCTION_DECL)
+ {
+ location_t loc = smallest_type_location (declspecs);
+ if (loc != UNKNOWN_LOCATION)
+ {
+ tree restype = TREE_TYPE (TREE_TYPE (decl));
+ tree resdecl = build_decl (loc, RESULT_DECL, 0, restype);
+ DECL_ARTIFICIAL (resdecl) = 1;
+ DECL_IGNORED_P (resdecl) = 1;
+ DECL_RESULT (decl) = resdecl;
+ }
+ }
/* Record constancy and volatility on the DECL itself . There's
no need to do this when processing a template; we'll do this
@@ -17328,9 +17340,19 @@ start_preparsed_function (tree decl1, tree attrs, int flags)
if (DECL_RESULT (decl1) == NULL_TREE)
{
- tree resdecl;
+ /* In a template instantiation, copy the return type location. When
+ parsing, the location will be set in grokdeclarator. */
+ location_t loc = input_location;
+ if (DECL_TEMPLATE_INSTANTIATION (decl1)
+ && !DECL_CXX_CONSTRUCTOR_P (decl1)
+ && !DECL_CXX_DESTRUCTOR_P (decl1))
+ {
+ tree tmpl = template_for_substitution (decl1);
+ tree res = DECL_RESULT (DECL_TEMPLATE_RESULT (tmpl));
+ loc = DECL_SOURCE_LOCATION (res);
+ }
- resdecl = build_decl (input_location, RESULT_DECL, 0, restype);
+ tree resdecl = build_decl (loc, RESULT_DECL, 0, restype);
DECL_ARTIFICIAL (resdecl) = 1;
DECL_IGNORED_P (resdecl) = 1;
DECL_RESULT (decl1) = resdecl;
diff --git a/gcc/function.cc b/gcc/function.cc
index 9c8773bbc59..dc333c27e92 100644
--- a/gcc/function.cc
+++ b/gcc/function.cc
@@ -4997,7 +4997,8 @@ init_function_start (tree subr)
/* Warn if this value is an aggregate type,
regardless of which calling convention we are using for it. */
if (AGGREGATE_TYPE_P (TREE_TYPE (DECL_RESULT (subr))))
- warning (OPT_Waggregate_return, "function returns an aggregate");
+ warning_at (DECL_SOURCE_LOCATION (DECL_RESULT (subr)),
+ OPT_Waggregate_return, "function returns an aggregate");
}
/* Expand code to verify the stack_protect_guard. This is invoked at
diff --git a/gcc/ipa-pure-const.cc b/gcc/ipa-pure-const.cc
index 572a6da274f..8f6e8f63d91 100644
--- a/gcc/ipa-pure-const.cc
+++ b/gcc/ipa-pure-const.cc
@@ -63,6 +63,7 @@ along with GCC; see the file COPYING3. If not see
#include "ipa-fnsummary.h"
#include "symtab-thunks.h"
#include "dbgcnt.h"
+#include "gcc-rich-location.h"
/* Lattice values for const and pure functions. Everything starts out
being const, then may drop to pure and then neither depending on
@@ -212,7 +213,18 @@ suggest_attribute (int option, tree decl, bool known_finite,
if (warned_about->contains (decl))
return warned_about;
warned_about->add (decl);
- warning_at (DECL_SOURCE_LOCATION (decl),
+
+ gcc_rich_location richloc (option == OPT_Wsuggest_attribute_noreturn
+ ? DECL_SOURCE_LOCATION (DECL_RESULT (decl))
+ : DECL_SOURCE_LOCATION (decl));
+ if (option == OPT_Wsuggest_attribute_noreturn)
+ {
+ richloc.add_fixit_replace
+ (IDENTIFIER_POINTER (TYPE_IDENTIFIER (void_type_node)));
+ richloc.add_fixit_insert_before (DECL_SOURCE_LOCATION (decl),
+ " __attribute__((__noreturn__)) ");
+ }
+ warning_at (&richloc,
option,
known_finite
? G_("function might be candidate for attribute %qs")
diff --git a/gcc/testsuite/c-c++-common/pr68833-1.c b/gcc/testsuite/c-c++-common/pr68833-1.c
index a6aefad5c98..b27b783d61a 100644
--- a/gcc/testsuite/c-c++-common/pr68833-1.c
+++ b/gcc/testsuite/c-c++-common/pr68833-1.c
@@ -15,7 +15,7 @@ f1 (const char *fmt)
extern void f2 (void);
void
-f2 (void) /* { dg-error "candidate for attribute 'noreturn'" "detect noreturn candidate" } */
+f2 (void) /* { dg-error "candidate for attribute 'noreturn'" "detect noreturn candidate" { target *-*-* } .-1 } */
{
__builtin_exit (0);
}
diff --git a/gcc/testsuite/g++.dg/diagnostic/return-type-loc1.C b/gcc/testsuite/g++.dg/diagnostic/return-type-loc1.C
new file mode 100644
index 00000000000..f96179ed4d5
--- /dev/null
+++ b/gcc/testsuite/g++.dg/diagnostic/return-type-loc1.C
@@ -0,0 +1,20 @@
+// { dg-do compile { target c++11 } }
+// { dg-additional-options -Waggregate-return }
+
+struct B { int i,j; };
+
+template <class T>
+struct A
+{
+ template <class U>
+ U // { dg-warning "aggregate" }
+ f() { return {}; }
+};
+
+int main()
+{
+ A<int>().f<B>(); // { dg-warning "aggregate" }
+}
+
+B // { dg-warning "aggregate" }
+g() { return {}; }
diff --git a/gcc/testsuite/g++.dg/other/resultdecl-1.C b/gcc/testsuite/g++.dg/other/resultdecl-1.C
new file mode 100644
index 00000000000..e3951c339a9
--- /dev/null
+++ b/gcc/testsuite/g++.dg/other/resultdecl-1.C
@@ -0,0 +1,32 @@
+/* Verify correct result decl location. */
+// { dg-options "-fdiagnostics-generate-patch -O1 -fipa-pure-const -Wsuggest-attribute=noreturn" }
+//
+// foo4: rem int, add: void attribute((noreturn))
+
+
+
+int
+foo4(void)
+{ while (1); }
+
+
+#if 0
+// { dg-warning "function might be candidate for attribute .noreturn." "" { target *-*-* } .-6 } // the fnname is at line .-5 though
+// prune the diff path..
+/* { dg-regexp "\\-\\-\\- .*" } */
+/* { dg-regexp "\\+\\+\\+ .*" } */
+{ dg-begin-multiline-output "" }
+@@ -5,8 +5,8 @@
+
+
+
+-int
+-foo4(void)
++void
++ __attribute__((__noreturn__)) foo4(void)
+ { while (1); }
+
+
+{ dg-end-multiline-output "" }
+#endif
+
diff --git a/gcc/testsuite/gcc.dg/noreturn-1.c b/gcc/testsuite/gcc.dg/noreturn-1.c
index cdbfb8dd667..a713ee924fc 100644
--- a/gcc/testsuite/gcc.dg/noreturn-1.c
+++ b/gcc/testsuite/gcc.dg/noreturn-1.c
@@ -25,7 +25,7 @@ foo3(void)
extern void foo4(void);
void
-foo4(void) /* { dg-warning "candidate for attribute 'noreturn'" "detect noreturn candidate" } */
+foo4(void) /* { dg-warning "candidate for attribute 'noreturn'" "detect noreturn candidate" { target *-*-* } .-1 } */
{
exit(0);
}
--
2.31.1
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 2/5] c++: Set the locus of the function result decl
2022-11-22 20:25 ` Jason Merrill
@ 2022-11-23 15:28 ` Jason Merrill
2022-12-02 19:30 ` Jason Merrill
0 siblings, 1 reply; 34+ messages in thread
From: Jason Merrill @ 2022-11-23 15:28 UTC (permalink / raw)
To: Bernhard Reutner-Fischer, gcc-patches; +Cc: Bernhard Reutner-Fischer
[-- Attachment #1: Type: text/plain, Size: 1064 bytes --]
On 11/22/22 15:25, Jason Merrill wrote:
> On 11/20/22 12:06, Bernhard Reutner-Fischer wrote:
>> Hi Jason!
>>
>> The "meh" of result-decl-plugin-test-2.C should likely be omitted,
>> grokdeclarator would need some changes to add richloc hints and we
>> would not
>> be able to make a reliable guess what to remove precisely.
>> C.f. /* Check all other uses of type modifiers. */
>> Furthermore it is unrelated to DECL_RESULT so not of direct interest
>> here. The other tests in test-2.C, f() and huh() should work though.
>>
>> I don't know if it's acceptable to change ipa-pure-const to make the
>> missing noreturn warning more precise and emit a fixit-hint. At least it
>> would be a real test for the DECL_RESULT and would spare us the plugin.
>
> The main problem I see with that change is that the syntax of the fixit
> might be wrong for non-C-family front-ends.
>
> Here's a version of the patch that fixes template/method handling, and
> adjusts -Waggregate-return as well:
Actually, that broke some of the spaceship tests, fixed by this version:
[-- Attachment #2: 0001-c-Set-the-locus-of-the-function-result-decl.patch --]
[-- Type: text/x-patch, Size: 9629 bytes --]
From 3c8106c95ec07d17ff5ade173126067c540ab7cc Mon Sep 17 00:00:00 2001
From: Bernhard Reutner-Fischer <rep.dot.nop@gmail.com>
Date: Sun, 20 Nov 2022 18:06:04 +0100
Subject: [PATCH] c++: Set the locus of the function result decl
To: gcc-patches@gcc.gnu.org
gcc/cp/ChangeLog:
* decl.cc (grokdeclarator): Build RESULT_DECL.
(start_preparsed_function): Copy location from template.
* semantics.cc (apply_deduced_return_type): Handle
arg != current_function_decl.
* method.cc (implicitly_declare_fn): Use it.
gcc/ChangeLog:
* function.cc (init_function_start): Use DECL_RESULT location
for -Waggregate-return warning.
* ipa-pure-const.cc (suggest_attribute): Add fixit-hint for the
noreturn attribute.
gcc/testsuite/ChangeLog:
* c-c++-common/pr68833-1.c: Adjust noreturn warning line number.
* gcc.dg/noreturn-1.c: Likewise.
* g++.dg/diagnostic/return-type-loc1.C: New test.
* g++.dg/other/resultdecl-1.C: New test.
Co-authored-by: Jason Merrill <jason@redhat.com>
---
gcc/cp/decl.cc | 26 +++++++++++++--
gcc/cp/method.cc | 2 +-
gcc/cp/semantics.cc | 15 ++++-----
gcc/function.cc | 3 +-
gcc/ipa-pure-const.cc | 14 +++++++-
gcc/testsuite/c-c++-common/pr68833-1.c | 2 +-
.../g++.dg/diagnostic/return-type-loc1.C | 20 ++++++++++++
gcc/testsuite/g++.dg/other/resultdecl-1.C | 32 +++++++++++++++++++
gcc/testsuite/gcc.dg/noreturn-1.c | 2 +-
9 files changed, 101 insertions(+), 15 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/diagnostic/return-type-loc1.C
create mode 100644 gcc/testsuite/g++.dg/other/resultdecl-1.C
diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
index 544efdc9914..2c5cd930e0a 100644
--- a/gcc/cp/decl.cc
+++ b/gcc/cp/decl.cc
@@ -14774,6 +14774,18 @@ grokdeclarator (const cp_declarator *declarator,
else if (constinit_p)
DECL_DECLARED_CONSTINIT_P (decl) = true;
}
+ else if (TREE_CODE (decl) == FUNCTION_DECL)
+ {
+ location_t loc = smallest_type_location (declspecs);
+ if (loc != UNKNOWN_LOCATION)
+ {
+ tree restype = TREE_TYPE (TREE_TYPE (decl));
+ tree resdecl = build_decl (loc, RESULT_DECL, 0, restype);
+ DECL_ARTIFICIAL (resdecl) = 1;
+ DECL_IGNORED_P (resdecl) = 1;
+ DECL_RESULT (decl) = resdecl;
+ }
+ }
/* Record constancy and volatility on the DECL itself . There's
no need to do this when processing a template; we'll do this
@@ -17328,9 +17340,19 @@ start_preparsed_function (tree decl1, tree attrs, int flags)
if (DECL_RESULT (decl1) == NULL_TREE)
{
- tree resdecl;
+ /* In a template instantiation, copy the return type location. When
+ parsing, the location will be set in grokdeclarator. */
+ location_t loc = input_location;
+ if (DECL_TEMPLATE_INSTANTIATION (decl1)
+ && !DECL_CXX_CONSTRUCTOR_P (decl1)
+ && !DECL_CXX_DESTRUCTOR_P (decl1))
+ {
+ tree tmpl = template_for_substitution (decl1);
+ tree res = DECL_RESULT (DECL_TEMPLATE_RESULT (tmpl));
+ loc = DECL_SOURCE_LOCATION (res);
+ }
- resdecl = build_decl (input_location, RESULT_DECL, 0, restype);
+ tree resdecl = build_decl (loc, RESULT_DECL, 0, restype);
DECL_ARTIFICIAL (resdecl) = 1;
DECL_IGNORED_P (resdecl) = 1;
DECL_RESULT (decl1) = resdecl;
diff --git a/gcc/cp/method.cc b/gcc/cp/method.cc
index 1e962b6e3b1..7b4d5a59823 100644
--- a/gcc/cp/method.cc
+++ b/gcc/cp/method.cc
@@ -3079,7 +3079,7 @@ implicitly_declare_fn (special_function_kind kind, tree type,
{
fn = copy_operator_fn (pattern_fn, EQ_EXPR);
DECL_ARTIFICIAL (fn) = 1;
- TREE_TYPE (fn) = change_return_type (boolean_type_node, TREE_TYPE (fn));
+ apply_deduced_return_type (fn, boolean_type_node);
return fn;
}
diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc
index 9401b35a789..ab52e56d6c1 100644
--- a/gcc/cp/semantics.cc
+++ b/gcc/cp/semantics.cc
@@ -12325,24 +12325,23 @@ apply_deduced_return_type (tree fco, tree return_type)
/* We already have a DECL_RESULT from start_preparsed_function.
Now we need to redo the work it and allocate_struct_function
did to reflect the new type. */
- gcc_assert (current_function_decl == fco);
- result = build_decl (input_location, RESULT_DECL, NULL_TREE,
+ result = build_decl (DECL_SOURCE_LOCATION (result), RESULT_DECL, NULL_TREE,
TYPE_MAIN_VARIANT (return_type));
DECL_ARTIFICIAL (result) = 1;
DECL_IGNORED_P (result) = 1;
cp_apply_type_quals_to_decl (cp_type_quals (return_type),
result);
-
DECL_RESULT (fco) = result;
if (!processing_template_decl)
- {
- bool aggr = aggregate_value_p (result, fco);
+ if (function *fun = DECL_STRUCT_FUNCTION (fco))
+ {
+ bool aggr = aggregate_value_p (result, fco);
#ifdef PCC_STATIC_STRUCT_RETURN
- cfun->returns_pcc_struct = aggr;
+ fun->returns_pcc_struct = aggr;
#endif
- cfun->returns_struct = aggr;
- }
+ fun->returns_struct = aggr;
+ }
}
/* DECL is a local variable or parameter from the surrounding scope of a
diff --git a/gcc/function.cc b/gcc/function.cc
index 9c8773bbc59..dc333c27e92 100644
--- a/gcc/function.cc
+++ b/gcc/function.cc
@@ -4997,7 +4997,8 @@ init_function_start (tree subr)
/* Warn if this value is an aggregate type,
regardless of which calling convention we are using for it. */
if (AGGREGATE_TYPE_P (TREE_TYPE (DECL_RESULT (subr))))
- warning (OPT_Waggregate_return, "function returns an aggregate");
+ warning_at (DECL_SOURCE_LOCATION (DECL_RESULT (subr)),
+ OPT_Waggregate_return, "function returns an aggregate");
}
/* Expand code to verify the stack_protect_guard. This is invoked at
diff --git a/gcc/ipa-pure-const.cc b/gcc/ipa-pure-const.cc
index 572a6da274f..8f6e8f63d91 100644
--- a/gcc/ipa-pure-const.cc
+++ b/gcc/ipa-pure-const.cc
@@ -63,6 +63,7 @@ along with GCC; see the file COPYING3. If not see
#include "ipa-fnsummary.h"
#include "symtab-thunks.h"
#include "dbgcnt.h"
+#include "gcc-rich-location.h"
/* Lattice values for const and pure functions. Everything starts out
being const, then may drop to pure and then neither depending on
@@ -212,7 +213,18 @@ suggest_attribute (int option, tree decl, bool known_finite,
if (warned_about->contains (decl))
return warned_about;
warned_about->add (decl);
- warning_at (DECL_SOURCE_LOCATION (decl),
+
+ gcc_rich_location richloc (option == OPT_Wsuggest_attribute_noreturn
+ ? DECL_SOURCE_LOCATION (DECL_RESULT (decl))
+ : DECL_SOURCE_LOCATION (decl));
+ if (option == OPT_Wsuggest_attribute_noreturn)
+ {
+ richloc.add_fixit_replace
+ (IDENTIFIER_POINTER (TYPE_IDENTIFIER (void_type_node)));
+ richloc.add_fixit_insert_before (DECL_SOURCE_LOCATION (decl),
+ " __attribute__((__noreturn__)) ");
+ }
+ warning_at (&richloc,
option,
known_finite
? G_("function might be candidate for attribute %qs")
diff --git a/gcc/testsuite/c-c++-common/pr68833-1.c b/gcc/testsuite/c-c++-common/pr68833-1.c
index a6aefad5c98..b27b783d61a 100644
--- a/gcc/testsuite/c-c++-common/pr68833-1.c
+++ b/gcc/testsuite/c-c++-common/pr68833-1.c
@@ -15,7 +15,7 @@ f1 (const char *fmt)
extern void f2 (void);
void
-f2 (void) /* { dg-error "candidate for attribute 'noreturn'" "detect noreturn candidate" } */
+f2 (void) /* { dg-error "candidate for attribute 'noreturn'" "detect noreturn candidate" { target *-*-* } .-1 } */
{
__builtin_exit (0);
}
diff --git a/gcc/testsuite/g++.dg/diagnostic/return-type-loc1.C b/gcc/testsuite/g++.dg/diagnostic/return-type-loc1.C
new file mode 100644
index 00000000000..f96179ed4d5
--- /dev/null
+++ b/gcc/testsuite/g++.dg/diagnostic/return-type-loc1.C
@@ -0,0 +1,20 @@
+// { dg-do compile { target c++11 } }
+// { dg-additional-options -Waggregate-return }
+
+struct B { int i,j; };
+
+template <class T>
+struct A
+{
+ template <class U>
+ U // { dg-warning "aggregate" }
+ f() { return {}; }
+};
+
+int main()
+{
+ A<int>().f<B>(); // { dg-warning "aggregate" }
+}
+
+B // { dg-warning "aggregate" }
+g() { return {}; }
diff --git a/gcc/testsuite/g++.dg/other/resultdecl-1.C b/gcc/testsuite/g++.dg/other/resultdecl-1.C
new file mode 100644
index 00000000000..e3951c339a9
--- /dev/null
+++ b/gcc/testsuite/g++.dg/other/resultdecl-1.C
@@ -0,0 +1,32 @@
+/* Verify correct result decl location. */
+// { dg-options "-fdiagnostics-generate-patch -O1 -fipa-pure-const -Wsuggest-attribute=noreturn" }
+//
+// foo4: rem int, add: void attribute((noreturn))
+
+
+
+int
+foo4(void)
+{ while (1); }
+
+
+#if 0
+// { dg-warning "function might be candidate for attribute .noreturn." "" { target *-*-* } .-6 } // the fnname is at line .-5 though
+// prune the diff path..
+/* { dg-regexp "\\-\\-\\- .*" } */
+/* { dg-regexp "\\+\\+\\+ .*" } */
+{ dg-begin-multiline-output "" }
+@@ -5,8 +5,8 @@
+
+
+
+-int
+-foo4(void)
++void
++ __attribute__((__noreturn__)) foo4(void)
+ { while (1); }
+
+
+{ dg-end-multiline-output "" }
+#endif
+
diff --git a/gcc/testsuite/gcc.dg/noreturn-1.c b/gcc/testsuite/gcc.dg/noreturn-1.c
index cdbfb8dd667..a713ee924fc 100644
--- a/gcc/testsuite/gcc.dg/noreturn-1.c
+++ b/gcc/testsuite/gcc.dg/noreturn-1.c
@@ -25,7 +25,7 @@ foo3(void)
extern void foo4(void);
void
-foo4(void) /* { dg-warning "candidate for attribute 'noreturn'" "detect noreturn candidate" } */
+foo4(void) /* { dg-warning "candidate for attribute 'noreturn'" "detect noreturn candidate" { target *-*-* } .-1 } */
{
exit(0);
}
--
2.31.1
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 2/5] c++: Set the locus of the function result decl
2022-11-23 15:28 ` Jason Merrill
@ 2022-12-02 19:30 ` Jason Merrill
2022-12-02 19:55 ` Bernhard Reutner-Fischer
0 siblings, 1 reply; 34+ messages in thread
From: Jason Merrill @ 2022-12-02 19:30 UTC (permalink / raw)
To: Bernhard Reutner-Fischer, gcc-patches; +Cc: Bernhard Reutner-Fischer
[-- Attachment #1: Type: text/plain, Size: 1155 bytes --]
On 11/23/22 10:28, Jason Merrill wrote:
> On 11/22/22 15:25, Jason Merrill wrote:
>> On 11/20/22 12:06, Bernhard Reutner-Fischer wrote:
>>> Hi Jason!
>>>
>>> The "meh" of result-decl-plugin-test-2.C should likely be omitted,
>>> grokdeclarator would need some changes to add richloc hints and we
>>> would not
>>> be able to make a reliable guess what to remove precisely.
>>> C.f. /* Check all other uses of type modifiers. */
>>> Furthermore it is unrelated to DECL_RESULT so not of direct interest
>>> here. The other tests in test-2.C, f() and huh() should work though.
>>>
>>> I don't know if it's acceptable to change ipa-pure-const to make the
>>> missing noreturn warning more precise and emit a fixit-hint. At least it
>>> would be a real test for the DECL_RESULT and would spare us the plugin.
>>
>> The main problem I see with that change is that the syntax of the
>> fixit might be wrong for non-C-family front-ends.
>>
>> Here's a version of the patch that fixes template/method handling, and
>> adjusts -Waggregate-return as well:
>
> Actually, that broke some of the spaceship tests, fixed by this version:
Here's what I'm applying:
[-- Attachment #2: 0001-c-Set-the-locus-of-the-function-result-decl.patch --]
[-- Type: text/x-patch, Size: 5894 bytes --]
From d19aa6af6634b1e97f38431ad091f3b3f12baf2f Mon Sep 17 00:00:00 2001
From: Bernhard Reutner-Fischer <rep.dot.nop@gmail.com>
Date: Sun, 20 Nov 2022 18:06:04 +0100
Subject: [PATCH] c++: Set the locus of the function result decl
To: gcc-patches@gcc.gnu.org
gcc/cp/ChangeLog:
* decl.cc (grokdeclarator): Build RESULT_DECL.
(start_preparsed_function): Copy location from template.
* semantics.cc (apply_deduced_return_type): Handle
arg != current_function_decl.
* method.cc (implicitly_declare_fn): Use it.
gcc/ChangeLog:
* function.cc (init_function_start): Use DECL_RESULT location
for -Waggregate-return warning.
gcc/testsuite/ChangeLog:
* g++.dg/diagnostic/return-type-loc1.C: New test.
Co-authored-by: Jason Merrill <jason@redhat.com>
---
gcc/cp/decl.cc | 25 +++++++++++++++++--
gcc/cp/method.cc | 2 +-
gcc/cp/semantics.cc | 15 ++++++-----
gcc/function.cc | 3 ++-
.../g++.dg/diagnostic/return-type-loc1.C | 20 +++++++++++++++
5 files changed, 53 insertions(+), 12 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/diagnostic/return-type-loc1.C
diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
index 238e72f90da..508156309d9 100644
--- a/gcc/cp/decl.cc
+++ b/gcc/cp/decl.cc
@@ -14772,6 +14772,19 @@ grokdeclarator (const cp_declarator *declarator,
else if (constinit_p)
DECL_DECLARED_CONSTINIT_P (decl) = true;
}
+ else if (TREE_CODE (decl) == FUNCTION_DECL)
+ {
+ /* If we saw a return type, record its location. */
+ location_t loc = declspecs->locations[ds_type_spec];
+ if (loc != UNKNOWN_LOCATION)
+ {
+ tree restype = TREE_TYPE (TREE_TYPE (decl));
+ tree resdecl = build_decl (loc, RESULT_DECL, 0, restype);
+ DECL_ARTIFICIAL (resdecl) = 1;
+ DECL_IGNORED_P (resdecl) = 1;
+ DECL_RESULT (decl) = resdecl;
+ }
+ }
/* Record constancy and volatility on the DECL itself . There's
no need to do this when processing a template; we'll do this
@@ -17326,9 +17339,17 @@ start_preparsed_function (tree decl1, tree attrs, int flags)
if (DECL_RESULT (decl1) == NULL_TREE)
{
- tree resdecl;
+ /* In a template instantiation, copy the return type location. When
+ parsing, the location will be set in grokdeclarator. */
+ location_t loc = input_location;
+ if (DECL_TEMPLATE_INSTANTIATION (decl1))
+ {
+ tree tmpl = template_for_substitution (decl1);
+ if (tree res = DECL_RESULT (DECL_TEMPLATE_RESULT (tmpl)))
+ loc = DECL_SOURCE_LOCATION (res);
+ }
- resdecl = build_decl (input_location, RESULT_DECL, 0, restype);
+ tree resdecl = build_decl (loc, RESULT_DECL, 0, restype);
DECL_ARTIFICIAL (resdecl) = 1;
DECL_IGNORED_P (resdecl) = 1;
DECL_RESULT (decl1) = resdecl;
diff --git a/gcc/cp/method.cc b/gcc/cp/method.cc
index 1e962b6e3b1..7b4d5a59823 100644
--- a/gcc/cp/method.cc
+++ b/gcc/cp/method.cc
@@ -3079,7 +3079,7 @@ implicitly_declare_fn (special_function_kind kind, tree type,
{
fn = copy_operator_fn (pattern_fn, EQ_EXPR);
DECL_ARTIFICIAL (fn) = 1;
- TREE_TYPE (fn) = change_return_type (boolean_type_node, TREE_TYPE (fn));
+ apply_deduced_return_type (fn, boolean_type_node);
return fn;
}
diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc
index 9401b35a789..ab52e56d6c1 100644
--- a/gcc/cp/semantics.cc
+++ b/gcc/cp/semantics.cc
@@ -12325,24 +12325,23 @@ apply_deduced_return_type (tree fco, tree return_type)
/* We already have a DECL_RESULT from start_preparsed_function.
Now we need to redo the work it and allocate_struct_function
did to reflect the new type. */
- gcc_assert (current_function_decl == fco);
- result = build_decl (input_location, RESULT_DECL, NULL_TREE,
+ result = build_decl (DECL_SOURCE_LOCATION (result), RESULT_DECL, NULL_TREE,
TYPE_MAIN_VARIANT (return_type));
DECL_ARTIFICIAL (result) = 1;
DECL_IGNORED_P (result) = 1;
cp_apply_type_quals_to_decl (cp_type_quals (return_type),
result);
-
DECL_RESULT (fco) = result;
if (!processing_template_decl)
- {
- bool aggr = aggregate_value_p (result, fco);
+ if (function *fun = DECL_STRUCT_FUNCTION (fco))
+ {
+ bool aggr = aggregate_value_p (result, fco);
#ifdef PCC_STATIC_STRUCT_RETURN
- cfun->returns_pcc_struct = aggr;
+ fun->returns_pcc_struct = aggr;
#endif
- cfun->returns_struct = aggr;
- }
+ fun->returns_struct = aggr;
+ }
}
/* DECL is a local variable or parameter from the surrounding scope of a
diff --git a/gcc/function.cc b/gcc/function.cc
index 9c8773bbc59..dc333c27e92 100644
--- a/gcc/function.cc
+++ b/gcc/function.cc
@@ -4997,7 +4997,8 @@ init_function_start (tree subr)
/* Warn if this value is an aggregate type,
regardless of which calling convention we are using for it. */
if (AGGREGATE_TYPE_P (TREE_TYPE (DECL_RESULT (subr))))
- warning (OPT_Waggregate_return, "function returns an aggregate");
+ warning_at (DECL_SOURCE_LOCATION (DECL_RESULT (subr)),
+ OPT_Waggregate_return, "function returns an aggregate");
}
/* Expand code to verify the stack_protect_guard. This is invoked at
diff --git a/gcc/testsuite/g++.dg/diagnostic/return-type-loc1.C b/gcc/testsuite/g++.dg/diagnostic/return-type-loc1.C
new file mode 100644
index 00000000000..f96179ed4d5
--- /dev/null
+++ b/gcc/testsuite/g++.dg/diagnostic/return-type-loc1.C
@@ -0,0 +1,20 @@
+// { dg-do compile { target c++11 } }
+// { dg-additional-options -Waggregate-return }
+
+struct B { int i,j; };
+
+template <class T>
+struct A
+{
+ template <class U>
+ U // { dg-warning "aggregate" }
+ f() { return {}; }
+};
+
+int main()
+{
+ A<int>().f<B>(); // { dg-warning "aggregate" }
+}
+
+B // { dg-warning "aggregate" }
+g() { return {}; }
--
2.31.1
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 2/5] c++: Set the locus of the function result decl
2022-12-02 19:30 ` Jason Merrill
@ 2022-12-02 19:55 ` Bernhard Reutner-Fischer
0 siblings, 0 replies; 34+ messages in thread
From: Bernhard Reutner-Fischer @ 2022-12-02 19:55 UTC (permalink / raw)
To: Jason Merrill, gcc-patches; +Cc: Bernhard Reutner-Fischer
On 2 December 2022 20:30:56 CET, Jason Merrill <jason@redhat.com> wrote:
>Here's what I'm applying:
I meant to play with it during the weekend,
looks good, many thanks for taking care of this!
cheers,
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v2] Fortran: Narrow return types [PR78798]
2022-11-13 21:50 ` Bernhard Reutner-Fischer
@ 2023-05-10 16:47 ` Bernhard Reutner-Fischer
2023-05-14 12:27 ` Mikael Morin
0 siblings, 1 reply; 34+ messages in thread
From: Bernhard Reutner-Fischer @ 2023-05-10 16:47 UTC (permalink / raw)
To: fortran; +Cc: gcc-patches, Bernhard Reutner-Fischer
From: Bernhard Reutner-Fischer <aldot@gcc.gnu.org>
gcc/fortran/ChangeLog:
PR fortran/78798
* array.cc (compare_bounds): Use narrower return type.
(gfc_compare_array_spec): Likewise.
(is_constant_element): Likewise.
(gfc_constant_ac): Likewise.
* check.cc (dim_rank_check): Likewise.
* cpp.cc (gfc_cpp_init_options): Likewise.
(dump_macro): Likewise.
* cpp.h (gfc_cpp_handle_option): Likewise.
* dependency.cc (gfc_ref_needs_temporary_p): Likewise.
(gfc_check_argument_dependency): Likewise.
(gfc_check_fncall_dependency): Likewise.
(ref_same_as_full_array): Likewise.
* dependency.h (gfc_check_fncall_dependency): Likewise.
(gfc_dep_resolver): Likewise.
(gfc_are_equivalenced_arrays): Likewise.
* expr.cc (gfc_copy_ref): Likewise.
(gfc_kind_max): Likewise.
(numeric_type): Likewise.
* gfortran.h (gfc_at_end): Likewise.
(gfc_at_eof): Likewise.
(gfc_at_bol): Likewise.
(gfc_at_eol): Likewise.
(gfc_define_undef_line): Likewise.
(gfc_wide_is_printable): Likewise.
(gfc_wide_is_digit): Likewise.
(gfc_wide_fits_in_byte): Likewise.
(gfc_find_sym_tree): Likewise.
(gfc_generic_intrinsic): Likewise.
(gfc_specific_intrinsic): Likewise.
(gfc_intrinsic_actual_ok): Likewise.
(gfc_has_vector_index): Likewise.
(gfc_numeric_ts): Likewise.
(gfc_impure_variable): Likewise.
(gfc_pure): Likewise.
(gfc_implicit_pure): Likewise.
(gfc_elemental): Likewise.
(gfc_pure_function): Likewise.
(gfc_implicit_pure_function): Likewise.
(gfc_compare_array_spec): Likewise.
(gfc_constant_ac): Likewise.
(gfc_expanded_ac): Likewise.
(gfc_check_digit): Likewise.
* intrinsic.cc (gfc_find_subroutine): Likewise.
(gfc_generic_intrinsic): Likewise.
(gfc_specific_intrinsic): Likewise.
* io.cc (compare_to_allowed_values): Likewise. And remove
unneeded forward declaration.
* misc.cc (gfc_done_2): Likewise.
* parse.cc: Likewise.
* parse.h (gfc_check_do_variable): Likewise.
* primary.cc (gfc_check_digit): Likewise.
* resolve.cc (resolve_structure_cons): Likewise.
(pure_stmt_function): Likewise.
(gfc_pure_function): Likewise.
(impure_stmt_fcn): Likewise.
(resolve_forall_iterators): Likewise.
(resolve_data): Likewise.
(gfc_impure_variable): Likewise.
(gfc_pure): Likewise.
(gfc_unset_implicit_pure): Likewise.
* scanner.cc (wide_is_ascii): Likewise.
(gfc_wide_toupper): Likewise.
(gfc_open_included_file): Likewise.
(gfc_at_end): Likewise.
(gfc_at_eof): Likewise.
(gfc_at_bol): Likewise.
(skip_comment_line): Likewise.
(gfc_gobble_whitespace): Likewise.
* symbol.cc (gfc_find_symtree_in_proc): Likewise.
* trans-array.cc: Likewise.
* trans-decl.cc (gfc_set_decl_assembler_name): Likewise.
* trans-types.cc (gfc_get_element_type): Likewise.
(gfc_add_field_to_struct): Likewise.
* trans-types.h (gfc_copy_dt_decls_ifequal): Likewise.
(gfc_return_by_reference): Likewise.
(gfc_is_nodesc_array): Likewise.
* trans.h (gfc_can_put_var_on_stack): Likewise.
---
Bootstrapped without new warnings and regression tested on
x86_64-linux with no regressions, OK for trunk?
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 | 48 +++++++++++++++++++-------------------
gcc/fortran/intrinsic.cc | 6 ++---
gcc/fortran/io.cc | 13 ++---------
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/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 +-
21 files changed, 81 insertions(+), 91 deletions(-)
diff --git a/gcc/fortran/array.cc b/gcc/fortran/array.cc
index be5eb8b6a0f..4b7c1e715bf 100644
--- a/gcc/fortran/array.cc
+++ b/gcc/fortran/array.cc
@@ -994,7 +994,7 @@ compare_bounds (gfc_expr *bound1, gfc_expr *bound2)
/* Compares two array specifications. They must be constant or deferred
shape. */
-int
+bool
gfc_compare_array_spec (gfc_array_spec *as1, gfc_array_spec *as2)
{
int i;
@@ -1039,7 +1039,7 @@ gfc_compare_array_spec (gfc_array_spec *as1, gfc_array_spec *as2)
use the symbol as an implied-DO iterator. Returns nonzero if a
duplicate was found. */
-static int
+static bool
check_duplicate_iterator (gfc_constructor_base base, gfc_symbol *master)
{
gfc_constructor *c;
@@ -1982,7 +1982,7 @@ is_constant_element (gfc_expr *e)
i=1,100000000) /) will take a while as* opposed to a more clever
function that traverses the expression tree. FIXME. */
-int
+bool
gfc_constant_ac (gfc_expr *e)
{
expand_info expand_save;
@@ -2005,7 +2005,7 @@ gfc_constant_ac (gfc_expr *e)
/* Returns nonzero if an array constructor has been completely
expanded (no iterators) and zero if iterators are present. */
-int
+bool
gfc_expanded_ac (gfc_expr *e)
{
gfc_constructor *c;
diff --git a/gcc/fortran/check.cc b/gcc/fortran/check.cc
index b348bda6e6c..4e3aed84b9d 100644
--- a/gcc/fortran/check.cc
+++ b/gcc/fortran/check.cc
@@ -1156,7 +1156,7 @@ dim_rank_check (gfc_expr *dim, gfc_expr *array, int allow_assumed)
dimension bi, returning 0 if they are known not to be identical,
and 1 if they are identical, or if this cannot be determined. */
-static int
+static bool
identical_dimen_shape (gfc_expr *a, int ai, gfc_expr *b, int bi)
{
mpz_t a_size, b_size;
diff --git a/gcc/fortran/cpp.cc b/gcc/fortran/cpp.cc
index c3b7c7f7bd9..d7890a97287 100644
--- a/gcc/fortran/cpp.cc
+++ b/gcc/fortran/cpp.cc
@@ -297,7 +297,7 @@ gfc_cpp_init_options (unsigned int decoded_options_count,
gfc_cpp_option.deferred_opt_count = 0;
}
-int
+bool
gfc_cpp_handle_option (size_t scode, const char *arg, int value ATTRIBUTE_UNUSED)
{
int result = 1;
@@ -749,7 +749,6 @@ gfc_cpp_add_include_path_after (char *path, bool user_supplied)
static void scan_translation_unit_trad (cpp_reader *);
static void account_for_newlines (const unsigned char *, size_t);
-static int dump_macro (cpp_reader *, cpp_hashnode *, void *);
static void print_line (location_t, const char *);
static void maybe_print_line (location_t);
diff --git a/gcc/fortran/cpp.h b/gcc/fortran/cpp.h
index 886996f2784..d2fbfcf9525 100644
--- a/gcc/fortran/cpp.h
+++ b/gcc/fortran/cpp.h
@@ -39,7 +39,7 @@ void gfc_cpp_init (void);
void gfc_cpp_init_options (unsigned int decoded_options_count,
struct cl_decoded_option *decoded_options);
-int gfc_cpp_handle_option(size_t scode, const char *arg, int value);
+bool gfc_cpp_handle_option(size_t scode, const char *arg, int value);
void gfc_cpp_post_options (bool);
diff --git a/gcc/fortran/dependency.cc b/gcc/fortran/dependency.cc
index a648d5c7903..b398b29a642 100644
--- a/gcc/fortran/dependency.cc
+++ b/gcc/fortran/dependency.cc
@@ -921,7 +921,7 @@ gfc_ref_needs_temporary_p (gfc_ref *ref)
}
-static int
+static bool
gfc_is_data_pointer (gfc_expr *e)
{
gfc_ref *ref;
@@ -1091,7 +1091,7 @@ gfc_check_argument_dependency (gfc_expr *other, sym_intent intent,
/* Like gfc_check_argument_dependency, but check all the arguments in ACTUAL.
FNSYM is the function being called, or NULL if not known. */
-int
+bool
gfc_check_fncall_dependency (gfc_expr *other, sym_intent intent,
gfc_symbol *fnsym, gfc_actual_arglist *actual,
gfc_dep_check elemental)
@@ -1137,7 +1137,7 @@ gfc_check_fncall_dependency (gfc_expr *other, sym_intent intent,
e1->ref and e2->ref to determine whether the actually accessed
portions of these variables/arrays potentially overlap. */
-int
+bool
gfc_are_equivalenced_arrays (gfc_expr *e1, gfc_expr *e2)
{
gfc_equiv_list *l;
@@ -2098,7 +2098,7 @@ ref_same_as_full_array (gfc_ref *full_ref, gfc_ref *ref)
there is some kind of overlap.
0 : array references are identical or not overlapping. */
-int
+bool
gfc_dep_resolver (gfc_ref *lref, gfc_ref *rref, gfc_reverse *reverse,
bool identical)
{
diff --git a/gcc/fortran/dependency.h b/gcc/fortran/dependency.h
index b1150316f5d..fbbede8b22c 100644
--- a/gcc/fortran/dependency.h
+++ b/gcc/fortran/dependency.h
@@ -32,13 +32,13 @@ enum gfc_dep_check
bool gfc_ref_needs_temporary_p (gfc_ref *);
bool gfc_full_array_ref_p (gfc_ref *, bool *);
gfc_expr *gfc_get_noncopying_intrinsic_argument (gfc_expr *);
-int gfc_check_fncall_dependency (gfc_expr *, sym_intent, gfc_symbol *,
+bool gfc_check_fncall_dependency (gfc_expr *, sym_intent, gfc_symbol *,
gfc_actual_arglist *, gfc_dep_check);
int gfc_check_dependency (gfc_expr *, gfc_expr *, bool);
int gfc_expr_is_one (gfc_expr *, int);
-int gfc_dep_resolver (gfc_ref *, gfc_ref *, gfc_reverse *,
+bool gfc_dep_resolver (gfc_ref *, gfc_ref *, gfc_reverse *,
bool identical = false);
-int gfc_are_equivalenced_arrays (gfc_expr *, gfc_expr *);
+bool gfc_are_equivalenced_arrays (gfc_expr *, gfc_expr *);
gfc_expr * gfc_discard_nops (gfc_expr *);
diff --git a/gcc/fortran/expr.cc b/gcc/fortran/expr.cc
index d91722e6ac6..aa01a4d3d22 100644
--- a/gcc/fortran/expr.cc
+++ b/gcc/fortran/expr.cc
@@ -798,7 +798,7 @@ gfc_copy_ref (gfc_ref *src)
/* Detect whether an expression has any vector index array references. */
-int
+bool
gfc_has_vector_index (gfc_expr *e)
{
gfc_ref *ref;
@@ -888,7 +888,7 @@ gfc_kind_max (gfc_expr *e1, gfc_expr *e2)
/* Returns nonzero if the type is numeric, zero otherwise. */
-static int
+static bool
numeric_type (bt type)
{
return type == BT_COMPLEX || type == BT_REAL || type == BT_INTEGER;
@@ -897,7 +897,7 @@ numeric_type (bt type)
/* Returns nonzero if the typespec is a numeric type, zero otherwise. */
-int
+bool
gfc_numeric_ts (gfc_typespec *ts)
{
return numeric_type (ts->type);
diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h
index ac21e1813d9..9dd6b45f112 100644
--- a/gcc/fortran/gfortran.h
+++ b/gcc/fortran/gfortran.h
@@ -3220,16 +3220,16 @@ void gfc_release_include_path (void);
void gfc_check_include_dirs (bool);
FILE *gfc_open_included_file (const char *, bool, bool);
-int gfc_at_end (void);
-int gfc_at_eof (void);
-int gfc_at_bol (void);
-int gfc_at_eol (void);
+bool gfc_at_end (void);
+bool gfc_at_eof (void);
+bool gfc_at_bol (void);
+bool gfc_at_eol (void);
void gfc_advance_line (void);
-int gfc_define_undef_line (void);
+bool gfc_define_undef_line (void);
-int gfc_wide_is_printable (gfc_char_t);
-int gfc_wide_is_digit (gfc_char_t);
-int gfc_wide_fits_in_byte (gfc_char_t);
+bool gfc_wide_is_printable (gfc_char_t);
+bool gfc_wide_is_digit (gfc_char_t);
+bool gfc_wide_fits_in_byte (gfc_char_t);
gfc_char_t gfc_wide_tolower (gfc_char_t);
gfc_char_t gfc_wide_toupper (gfc_char_t);
size_t gfc_wide_strlen (const gfc_char_t *);
@@ -3487,7 +3487,7 @@ void gfc_release_symbol (gfc_symbol *&);
gfc_symbol *gfc_new_symbol (const char *, gfc_namespace *);
gfc_symtree* gfc_find_symtree_in_proc (const char *, gfc_namespace *);
int gfc_find_symbol (const char *, gfc_namespace *, int, gfc_symbol **);
-int gfc_find_sym_tree (const char *, gfc_namespace *, int, gfc_symtree **);
+bool gfc_find_sym_tree (const char *, gfc_namespace *, int, gfc_symtree **);
int gfc_get_symbol (const char *, gfc_namespace *, gfc_symbol **);
bool gfc_verify_c_interop (gfc_typespec *);
bool gfc_verify_c_interop_param (gfc_symbol *);
@@ -3567,10 +3567,10 @@ bool gfc_convert_type (gfc_expr *, gfc_typespec *, int);
bool gfc_convert_type_warn (gfc_expr *, gfc_typespec *, int, int,
bool array = false);
bool gfc_convert_chartype (gfc_expr *, gfc_typespec *);
-int gfc_generic_intrinsic (const char *);
-int gfc_specific_intrinsic (const char *);
+bool gfc_generic_intrinsic (const char *);
+bool gfc_specific_intrinsic (const char *);
bool gfc_is_intrinsic (gfc_symbol*, int, locus);
-int gfc_intrinsic_actual_ok (const char *, const bool);
+bool gfc_intrinsic_actual_ok (const char *, const bool);
gfc_intrinsic_sym *gfc_find_function (const char *);
gfc_intrinsic_sym *gfc_find_subroutine (const char *);
gfc_intrinsic_sym *gfc_intrinsic_function_by_id (gfc_isym_id);
@@ -3648,7 +3648,7 @@ void gfc_type_convert_binary (gfc_expr *, int);
bool gfc_is_constant_expr (gfc_expr *);
bool gfc_simplify_expr (gfc_expr *, int);
bool gfc_try_simplify_expr (gfc_expr *, int);
-int gfc_has_vector_index (gfc_expr *);
+bool gfc_has_vector_index (gfc_expr *);
gfc_expr *gfc_get_expr (void);
gfc_expr *gfc_get_array_expr (bt type, int kind, locus *);
@@ -3672,7 +3672,7 @@ gfc_ref* gfc_copy_ref (gfc_ref*);
bool gfc_specification_expr (gfc_expr *);
-int gfc_numeric_ts (gfc_typespec *);
+bool gfc_numeric_ts (gfc_typespec *);
int gfc_kind_max (gfc_expr *, gfc_expr *);
bool gfc_check_conformance (gfc_expr *, gfc_expr *, const char *, ...) ATTRIBUTE_PRINTF_3;
@@ -3737,11 +3737,11 @@ void gfc_resolve (gfc_namespace *);
void gfc_resolve_code (gfc_code *, gfc_namespace *);
void gfc_resolve_blocks (gfc_code *, gfc_namespace *);
void gfc_resolve_formal_arglist (gfc_symbol *);
-int gfc_impure_variable (gfc_symbol *);
-int gfc_pure (gfc_symbol *);
-int gfc_implicit_pure (gfc_symbol *);
+bool gfc_impure_variable (gfc_symbol *);
+bool gfc_pure (gfc_symbol *);
+bool gfc_implicit_pure (gfc_symbol *);
void gfc_unset_implicit_pure (gfc_symbol *);
-int gfc_elemental (gfc_symbol *);
+bool gfc_elemental (gfc_symbol *);
bool gfc_resolve_iterator (gfc_iterator *, bool, bool);
bool find_forall_index (gfc_expr *, gfc_symbol *, int);
bool gfc_resolve_index (gfc_expr *, int);
@@ -3755,8 +3755,8 @@ bool gfc_resolve_intrinsic (gfc_symbol *, locus *);
bool gfc_explicit_interface_required (gfc_symbol *, char *, int);
extern int gfc_do_concurrent_flag;
const char* gfc_lookup_function_fuzzy (const char *, gfc_symtree *);
-int gfc_pure_function (gfc_expr *e, const char **name);
-int gfc_implicit_pure_function (gfc_expr *e);
+bool gfc_pure_function (gfc_expr *e, const char **name);
+bool gfc_implicit_pure_function (gfc_expr *e);
/* array.cc */
@@ -3769,12 +3769,12 @@ bool gfc_set_array_spec (gfc_symbol *, gfc_array_spec *, locus *);
gfc_array_spec *gfc_copy_array_spec (gfc_array_spec *);
bool gfc_resolve_array_spec (gfc_array_spec *, int);
-int gfc_compare_array_spec (gfc_array_spec *, gfc_array_spec *);
+bool gfc_compare_array_spec (gfc_array_spec *, gfc_array_spec *);
void gfc_simplify_iterator_var (gfc_expr *);
bool gfc_expand_constructor (gfc_expr *, bool);
-int gfc_constant_ac (gfc_expr *);
-int gfc_expanded_ac (gfc_expr *);
+bool gfc_constant_ac (gfc_expr *);
+bool gfc_expanded_ac (gfc_expr *);
bool gfc_resolve_character_array_constructor (gfc_expr *);
bool gfc_resolve_array_constructor (gfc_expr *);
bool gfc_check_constructor_type (gfc_expr *);
@@ -3859,7 +3859,7 @@ symbol_attribute gfc_expr_attr (gfc_expr *);
symbol_attribute gfc_caf_attr (gfc_expr *, bool i = false, bool *r = NULL);
match gfc_match_rvalue (gfc_expr **);
match gfc_match_varspec (gfc_expr*, int, bool, bool);
-int gfc_check_digit (char, int);
+bool gfc_check_digit (char, int);
bool gfc_is_function_return_value (gfc_symbol *, gfc_namespace *);
bool gfc_convert_to_structure_constructor (gfc_expr *, gfc_symbol *,
gfc_expr **,
diff --git a/gcc/fortran/intrinsic.cc b/gcc/fortran/intrinsic.cc
index b5ef664eb33..74970e567fc 100644
--- a/gcc/fortran/intrinsic.cc
+++ b/gcc/fortran/intrinsic.cc
@@ -1107,7 +1107,7 @@ gfc_find_subroutine (const char *name)
/* Given a string, figure out if it is the name of a generic intrinsic
function or not. */
-int
+bool
gfc_generic_intrinsic (const char *name)
{
gfc_intrinsic_sym *sym;
@@ -1120,7 +1120,7 @@ gfc_generic_intrinsic (const char *name)
/* Given a string, figure out if it is the name of a specific
intrinsic function or not. */
-int
+bool
gfc_specific_intrinsic (const char *name)
{
gfc_intrinsic_sym *sym;
@@ -1132,7 +1132,7 @@ gfc_specific_intrinsic (const char *name)
/* Given a string, figure out if it is the name of an intrinsic function
or subroutine allowed as an actual argument or not. */
-int
+bool
gfc_intrinsic_actual_ok (const char *name, const bool subroutine_flag)
{
gfc_intrinsic_sym *sym;
diff --git a/gcc/fortran/io.cc b/gcc/fortran/io.cc
index 594f609cbf7..f0c605ebd80 100644
--- a/gcc/fortran/io.cc
+++ b/gcc/fortran/io.cc
@@ -2010,15 +2010,6 @@ gfc_free_open (gfc_open *open)
free (open);
}
-
-static int
-compare_to_allowed_values (const char *specifier, const char *allowed[],
- const char *allowed_f2003[],
- const char *allowed_gnu[], gfc_char_t *value,
- const char *statement, bool warn, locus *where,
- int *num = NULL);
-
-
static bool
check_open_constraints (gfc_open *open, locus *where);
@@ -2062,12 +2053,12 @@ gfc_resolve_open (gfc_open *open, locus *where)
value if it is not allowed. */
-static int
+static bool
compare_to_allowed_values (const char *specifier, const char *allowed[],
const char *allowed_f2003[],
const char *allowed_gnu[], gfc_char_t *value,
const char *statement, bool warn, locus *where,
- int *num)
+ int *num = NULL)
{
int i;
unsigned int len;
diff --git a/gcc/fortran/parse.cc b/gcc/fortran/parse.cc
index f1e55316e5b..5e2a95688d2 100644
--- a/gcc/fortran/parse.cc
+++ b/gcc/fortran/parse.cc
@@ -4710,7 +4710,7 @@ done:
context that causes it to become redefined. If the symbol is an
iterator, we generate an error message and return nonzero. */
-int
+bool
gfc_check_do_variable (gfc_symtree *st)
{
gfc_state_data *s;
diff --git a/gcc/fortran/parse.h b/gcc/fortran/parse.h
index 75aac8bb2a1..cc57e7d5729 100644
--- a/gcc/fortran/parse.h
+++ b/gcc/fortran/parse.h
@@ -63,7 +63,7 @@ extern gfc_state_data *gfc_state_stack;
#define gfc_comp_struct(s) \
((s) == COMP_DERIVED || (s) == COMP_STRUCTURE || (s) == COMP_MAP)
-int gfc_check_do_variable (gfc_symtree *);
+bool gfc_check_do_variable (gfc_symtree *);
bool gfc_find_state (gfc_compile_state);
gfc_state_data *gfc_enclosing_unit (gfc_compile_state *);
const char *gfc_ascii_statement (gfc_statement, bool strip_sentinel = false) ;
diff --git a/gcc/fortran/primary.cc b/gcc/fortran/primary.cc
index 077489ab96e..3099f82850c 100644
--- a/gcc/fortran/primary.cc
+++ b/gcc/fortran/primary.cc
@@ -109,10 +109,10 @@ get_kind (int *is_iso_c)
/* Given a character and a radix, see if the character is a valid
digit in that radix. */
-int
+bool
gfc_check_digit (char c, int radix)
{
- int r;
+ bool r;
switch (radix)
{
diff --git a/gcc/fortran/resolve.cc b/gcc/fortran/resolve.cc
index 58f05a3e74a..9c92958a397 100644
--- a/gcc/fortran/resolve.cc
+++ b/gcc/fortran/resolve.cc
@@ -1585,7 +1585,7 @@ resolve_structure_cons (gfc_expr *expr, int init)
/* Returns 0 if a symbol was not declared with a type or
attribute declaration statement, nonzero otherwise. */
-static int
+static bool
was_declared (gfc_symbol *sym)
{
symbol_attribute a;
@@ -3091,13 +3091,13 @@ is_external_proc (gfc_symbol *sym)
/* Figure out if a function reference is pure or not. Also set the name
of the function for a potential error message. Return nonzero if the
function is PURE, zero if not. */
-static int
+static bool
pure_stmt_function (gfc_expr *, gfc_symbol *);
-int
+bool
gfc_pure_function (gfc_expr *e, const char **name)
{
- int pure;
+ bool pure;
gfc_component *comp;
*name = NULL;
@@ -3137,7 +3137,7 @@ gfc_pure_function (gfc_expr *e, const char **name)
/* Check if the expression is a reference to an implicitly pure function. */
-int
+bool
gfc_implicit_pure_function (gfc_expr *e)
{
gfc_component *comp = gfc_get_proc_ptr_comp (e);
@@ -3168,7 +3168,7 @@ impure_stmt_fcn (gfc_expr *e, gfc_symbol *sym,
}
-static int
+static bool
pure_stmt_function (gfc_expr *e, gfc_symbol *sym)
{
return gfc_traverse_expr (e, sym, impure_stmt_fcn, 0) ? 0 : 1;
@@ -7630,7 +7630,7 @@ resolve_forall_iterators (gfc_forall_iterator *it)
PRIVATE. The search is recursive if necessary. Returns zero if no
inaccessible components are found, nonzero otherwise. */
-static int
+static bool
derived_inaccessible (gfc_symbol *sym)
{
gfc_component *c;
@@ -17126,7 +17126,7 @@ resolve_data (gfc_data *d)
/* Determines if a variable is not 'pure', i.e., not assignable within a pure
procedure. Returns zero if assignment is OK, nonzero if there is a
problem. */
-int
+bool
gfc_impure_variable (gfc_symbol *sym)
{
gfc_symbol *proc;
@@ -17161,7 +17161,7 @@ gfc_impure_variable (gfc_symbol *sym)
/* Test whether a symbol is pure or not. For a NULL pointer, checks if the
current namespace is inside a pure procedure. */
-int
+bool
gfc_pure (gfc_symbol *sym)
{
symbol_attribute attr;
@@ -17193,7 +17193,7 @@ gfc_pure (gfc_symbol *sym)
checks if the current namespace is implicitly pure. Note that this
function returns false for a PURE procedure. */
-int
+bool
gfc_implicit_pure (gfc_symbol *sym)
{
gfc_namespace *ns;
@@ -17247,7 +17247,7 @@ gfc_unset_implicit_pure (gfc_symbol *sym)
/* Test whether the current procedure is elemental or not. */
-int
+bool
gfc_elemental (gfc_symbol *sym)
{
symbol_attribute attr;
diff --git a/gcc/fortran/scanner.cc b/gcc/fortran/scanner.cc
index 4f45da44607..9f0d9a7dc5e 100644
--- a/gcc/fortran/scanner.cc
+++ b/gcc/fortran/scanner.cc
@@ -56,7 +56,7 @@ gfc_directorylist *include_dirs, *intrinsic_modules_dirs;
static gfc_file *file_head, *current_file;
-static int continue_flag, end_flag, gcc_attribute_flag;
+static bool continue_flag, end_flag, gcc_attribute_flag;
/* If !$omp/!$acc occurred in current comment line. */
static int openmp_flag, openacc_flag;
static int continue_count, continue_line;
@@ -86,7 +86,7 @@ static gfc_char_t *last_error_char;
/* Functions dealing with our wide characters (gfc_char_t) and
sequences of such characters. */
-int
+bool
gfc_wide_fits_in_byte (gfc_char_t c)
{
return (c <= UCHAR_MAX);
@@ -98,7 +98,7 @@ wide_is_ascii (gfc_char_t c)
return (gfc_wide_fits_in_byte (c) && ((unsigned char) c & ~0x7f) == 0);
}
-int
+bool
gfc_wide_is_printable (gfc_char_t c)
{
return (gfc_wide_fits_in_byte (c) && ISPRINT ((unsigned char) c));
@@ -116,7 +116,7 @@ gfc_wide_toupper (gfc_char_t c)
return (wide_is_ascii (c) ? (gfc_char_t) TOUPPER((unsigned char) c) : c);
}
-int
+bool
gfc_wide_is_digit (gfc_char_t c)
{
return (c >= '0' && c <= '9');
@@ -518,7 +518,7 @@ gfc_open_included_file (const char *name, bool include_cwd, bool module)
/* Test to see if we're at the end of the main source file. */
-int
+bool
gfc_at_end (void)
{
return end_flag;
@@ -527,7 +527,7 @@ gfc_at_end (void)
/* Test to see if we're at the end of the current file. */
-int
+bool
gfc_at_eof (void)
{
if (gfc_at_end ())
@@ -545,7 +545,7 @@ gfc_at_eof (void)
/* Test to see if we're at the beginning of a new line. */
-int
+bool
gfc_at_bol (void)
{
if (gfc_at_eof ())
@@ -557,7 +557,7 @@ gfc_at_bol (void)
/* Test to see if we're at the end of a line. */
-int
+bool
gfc_at_eol (void)
{
if (gfc_at_eof ())
@@ -702,7 +702,7 @@ skip_comment_line (void)
}
-int
+bool
gfc_define_undef_line (void)
{
char *tmp;
@@ -1803,7 +1803,7 @@ gfc_gobble_whitespace (void)
easily report line and column numbers consistent with other
parts of gfortran. */
-static int
+static bool
load_line (FILE *input, gfc_char_t **pbuf, int *pbuflen, const int *first_char)
{
int c, maxlen, i, preprocessor_flag, buflen = *pbuflen;
diff --git a/gcc/fortran/symbol.cc b/gcc/fortran/symbol.cc
index 221165d6dac..b4b36e27d75 100644
--- a/gcc/fortran/symbol.cc
+++ b/gcc/fortran/symbol.cc
@@ -3216,7 +3216,7 @@ gfc_find_symtree_in_proc (const char* name, gfc_namespace* ns)
any parent namespaces if requested by a nonzero parent_flag.
Returns nonzero if the name is ambiguous. */
-int
+bool
gfc_find_sym_tree (const char *name, gfc_namespace *ns, int parent_flag,
gfc_symtree **result)
{
diff --git a/gcc/fortran/trans-array.cc b/gcc/fortran/trans-array.cc
index 785cf504816..9f8aa09673a 100644
--- a/gcc/fortran/trans-array.cc
+++ b/gcc/fortran/trans-array.cc
@@ -4982,7 +4982,7 @@ done:
/* Return true if both symbols could refer to the same data object. Does
not take account of aliasing due to equivalence statements. */
-static int
+static bool
symbols_could_alias (gfc_symbol *lsym, gfc_symbol *rsym, bool lsym_pointer,
bool lsym_target, bool rsym_pointer, bool rsym_target)
{
diff --git a/gcc/fortran/trans-decl.cc b/gcc/fortran/trans-decl.cc
index 18e2b8b78b4..cd32542eb86 100644
--- a/gcc/fortran/trans-decl.cc
+++ b/gcc/fortran/trans-decl.cc
@@ -483,7 +483,7 @@ gfc_set_decl_assembler_name (tree decl, tree name)
/* Returns true if a variable of specified size should go on the stack. */
-int
+bool
gfc_can_put_var_on_stack (tree size)
{
unsigned HOST_WIDE_INT low;
diff --git a/gcc/fortran/trans-types.cc b/gcc/fortran/trans-types.cc
index fc5c221a301..d718f28cc86 100644
--- a/gcc/fortran/trans-types.cc
+++ b/gcc/fortran/trans-types.cc
@@ -1374,7 +1374,7 @@ gfc_get_element_type (tree type)
/* Returns true if the array sym does not require a descriptor. */
-int
+bool
gfc_is_nodesc_array (gfc_symbol * sym)
{
symbol_attribute *array_attr;
@@ -2451,7 +2451,7 @@ gfc_add_field_to_struct (tree context, tree name, tree type, tree **chain)
the two derived type symbols are "equal", as described
in 4.4.2 and resolved by gfc_compare_derived_types. */
-int
+bool
gfc_copy_dt_decls_ifequal (gfc_symbol *from, gfc_symbol *to,
bool from_gsym)
{
@@ -2940,7 +2940,7 @@ copy_derived_types:
}
-int
+bool
gfc_return_by_reference (gfc_symbol * sym)
{
if (!sym->attr.function)
diff --git a/gcc/fortran/trans-types.h b/gcc/fortran/trans-types.h
index 2dc692325cf..d5746ca4366 100644
--- a/gcc/fortran/trans-types.h
+++ b/gcc/fortran/trans-types.h
@@ -88,7 +88,7 @@ tree gfc_get_character_type_len_for_eltype (tree, tree);
tree gfc_sym_type (gfc_symbol *, bool is_bind_c_arg = false);
tree gfc_get_cfi_type (int dimen, bool restricted);
tree gfc_typenode_for_spec (gfc_typespec *, int c = 0);
-int gfc_copy_dt_decls_ifequal (gfc_symbol *, gfc_symbol *, bool);
+bool gfc_copy_dt_decls_ifequal (gfc_symbol *, gfc_symbol *, bool);
tree gfc_get_function_type (gfc_symbol *, gfc_actual_arglist *args = NULL,
const char *fnspec = NULL);
@@ -109,10 +109,10 @@ tree gfc_add_field_to_struct (tree, tree, tree, tree **);
void gfc_finish_type (tree);
/* Some functions have an extra parameter for the return value. */
-int gfc_return_by_reference (gfc_symbol *);
+bool gfc_return_by_reference (gfc_symbol *);
/* Returns true if the array sym does not require a descriptor. */
-int gfc_is_nodesc_array (gfc_symbol *);
+bool gfc_is_nodesc_array (gfc_symbol *);
/* Return the DTYPE for an array. */
tree gfc_get_dtype_rank_type (int, tree);
diff --git a/gcc/fortran/trans.h b/gcc/fortran/trans.h
index 1ad6d944fcf..0c8d004736d 100644
--- a/gcc/fortran/trans.h
+++ b/gcc/fortran/trans.h
@@ -670,7 +670,7 @@ void gfc_restore_sym (gfc_symbol *, gfc_saved_var *);
void gfc_set_decl_assembler_name (tree, tree);
/* Returns true if a variable of specified size should go on the stack. */
-int gfc_can_put_var_on_stack (tree);
+bool gfc_can_put_var_on_stack (tree);
/* Set GFC_DECL_SCALAR_* on decl from sym if needed. */
void gfc_finish_decl_attrs (tree, symbol_attribute *);
--
2.30.2
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2] Fortran: Narrow return types [PR78798]
2023-05-10 16:47 ` [PATCH v2] " Bernhard Reutner-Fischer
@ 2023-05-14 12:27 ` Mikael Morin
2023-05-14 13:04 ` Thomas Koenig
2023-05-18 19:52 ` Bernhard Reutner-Fischer
0 siblings, 2 replies; 34+ messages in thread
From: Mikael Morin @ 2023-05-14 12:27 UTC (permalink / raw)
To: Bernhard Reutner-Fischer, fortran; +Cc: gcc-patches, Bernhard Reutner-Fischer
Le 10/05/2023 à 18:47, Bernhard Reutner-Fischer via Fortran a écrit :
> From: Bernhard Reutner-Fischer <aldot@gcc.gnu.org>
>
> gcc/fortran/ChangeLog:
>
> PR fortran/78798
> * array.cc (compare_bounds): Use narrower return type.
> (gfc_compare_array_spec): Likewise.
> (is_constant_element): Likewise.
> (gfc_constant_ac): Likewise.
(...)
> ---
> Bootstrapped without new warnings and regression tested on
> x86_64-linux with no regressions, OK for trunk?
>
(...)
> diff --git a/gcc/fortran/check.cc b/gcc/fortran/check.cc
> index b348bda6e6c..4e3aed84b9d 100644
> --- a/gcc/fortran/check.cc
> +++ b/gcc/fortran/check.cc
> @@ -1156,7 +1156,7 @@ dim_rank_check (gfc_expr *dim, gfc_expr *array, int allow_assumed)
> dimension bi, returning 0 if they are known not to be identical,
> and 1 if they are identical, or if this cannot be determined. */
>
> -static int
> +static bool
> identical_dimen_shape (gfc_expr *a, int ai, gfc_expr *b, int bi)
> {
> mpz_t a_size, b_size;
To be consistent, please change as well the local variable "ret" used as
return value from int to bool.
> diff --git a/gcc/fortran/cpp.cc b/gcc/fortran/cpp.cc
> index c3b7c7f7bd9..d7890a97287 100644
> --- a/gcc/fortran/cpp.cc
> +++ b/gcc/fortran/cpp.cc
> @@ -297,7 +297,7 @@ gfc_cpp_init_options (unsigned int decoded_options_count,
> gfc_cpp_option.deferred_opt_count = 0;
> }
>
> -int
> +bool
> gfc_cpp_handle_option (size_t scode, const char *arg, int value ATTRIBUTE_UNUSED)
> {
> int result = 1;
Same here, change the type of variable "result".
(...)
> diff --git a/gcc/fortran/dependency.cc b/gcc/fortran/dependency.cc
> index a648d5c7903..b398b29a642 100644
> --- a/gcc/fortran/dependency.cc
> +++ b/gcc/fortran/dependency.cc
(...)
> @@ -1091,7 +1091,7 @@ gfc_check_argument_dependency (gfc_expr *other, sym_intent intent,
> /* Like gfc_check_argument_dependency, but check all the arguments in ACTUAL.
> FNSYM is the function being called, or NULL if not known. */
>
> -int
> +bool
> gfc_check_fncall_dependency (gfc_expr *other, sym_intent intent,
> gfc_symbol *fnsym, gfc_actual_arglist *actual,
> gfc_dep_check elemental)
Why not change the associated subfunctions
(gfc_check_argument_dependency, gfc_check_argument_var_dependency) as well ?
(...)
> @@ -2098,7 +2098,7 @@ ref_same_as_full_array (gfc_ref *full_ref, gfc_ref *ref)
> there is some kind of overlap.
> 0 : array references are identical or not overlapping. */
>
> -int
> +bool
> gfc_dep_resolver (gfc_ref *lref, gfc_ref *rref, gfc_reverse *reverse,
> bool identical)
> {
The function comment states that the function may return 2, which
doesn't seem to be the case any more. So please update the comment.
(...)> diff --git a/gcc/fortran/symbol.cc b/gcc/fortran/symbol.cc
> index 221165d6dac..b4b36e27d75 100644
> --- a/gcc/fortran/symbol.cc
> +++ b/gcc/fortran/symbol.cc
> @@ -3216,7 +3216,7 @@ gfc_find_symtree_in_proc (const char* name, gfc_namespace* ns)
> any parent namespaces if requested by a nonzero parent_flag.
> Returns nonzero if the name is ambiguous. */
>
> -int
> +bool
> gfc_find_sym_tree (const char *name, gfc_namespace *ns, int parent_flag,
> gfc_symtree **result)
> {
Maybe change nonzero to true in the comment?
(...)
OK with all the above fixed.
Thanks.
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2] Fortran: Narrow return types [PR78798]
2023-05-14 12:27 ` Mikael Morin
@ 2023-05-14 13:04 ` Thomas Koenig
2023-05-14 15:24 ` Bernhard Reutner-Fischer
2023-05-18 19:52 ` Bernhard Reutner-Fischer
1 sibling, 1 reply; 34+ messages in thread
From: Thomas Koenig @ 2023-05-14 13:04 UTC (permalink / raw)
To: Mikael Morin, Bernhard Reutner-Fischer, fortran
Cc: gcc-patches, Bernhard Reutner-Fischer
On 14.05.23 14:27, Mikael Morin wrote:
>
> (...)
>> @@ -2098,7 +2098,7 @@ ref_same_as_full_array (gfc_ref *full_ref,
>> gfc_ref *ref)
>> there is some kind of overlap.
>> 0 : array references are identical or not overlapping. */
>> -int
>> +bool
>> gfc_dep_resolver (gfc_ref *lref, gfc_ref *rref, gfc_reverse *reverse,
>> bool identical)
>> {
>
> The function comment states that the function may return 2, which
> doesn't seem to be the case any more.
Hm, this makes me a litte suspicious. Was functionality for reversing
loops lost, maybe unintentionally? I assume that, at some time, we did
use the '2' as return value (or did we?)
Best regards
Thomas
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2] Fortran: Narrow return types [PR78798]
2023-05-14 13:04 ` Thomas Koenig
@ 2023-05-14 15:24 ` Bernhard Reutner-Fischer
2023-05-14 18:06 ` Mikael Morin
0 siblings, 1 reply; 34+ messages in thread
From: Bernhard Reutner-Fischer @ 2023-05-14 15:24 UTC (permalink / raw)
To: Thomas Koenig
Cc: Mikael Morin, fortran, gcc-patches, Bernhard Reutner-Fischer
On Sun, 14 May 2023 15:04:15 +0200
Thomas Koenig <tkoenig@netcologne.de> wrote:
> On 14.05.23 14:27, Mikael Morin wrote:
> >
> > (...)
> >> @@ -2098,7 +2098,7 @@ ref_same_as_full_array (gfc_ref *full_ref,
> >> gfc_ref *ref)
> >> there is some kind of overlap.
> >> 0 : array references are identical or not overlapping. */
> >> -int
> >> +bool
> >> gfc_dep_resolver (gfc_ref *lref, gfc_ref *rref, gfc_reverse *reverse,
> >> bool identical)
> >> {
> >
> > The function comment states that the function may return 2, which
> > doesn't seem to be the case any more.
>
> Hm, this makes me a litte suspicious. Was functionality for reversing
> loops lost, maybe unintentionally? I assume that, at some time, we did
> use the '2' as return value (or did we?)
There was 7c428aa29d75ef163c334cf3974f87b3630d8b8b (a revert because it
miscompiled spec2k) which might have associated the comment of the
former static gfc_dependency dep_ref (gfc_ref *lref, gfc_ref *rref,
gfc_reverse *reverse) to the current gfc_dep_resolver.
The commit which introduced the return value 2 documentation was
3d03ead0b8273efde57f6194617b35111a84b05d
"re PR fortran/24524 (Fortran dependency checking should reverse loops)"
but TBH i don't see how it returned 2 in that revision?
Looks like when writing that patch it deemed useful to return 2 for
this specific situation but in the end it was dropped but the comment
survived.
I will update the comment to document the true / false return values.
And Mikael, do you want me to cleanup 1/0 to true/false assignments for
the boolean variables, or can we do that in a separate patch (or not at
all right now)?
many thanks for the reviews!
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2] Fortran: Narrow return types [PR78798]
2023-05-14 15:24 ` Bernhard Reutner-Fischer
@ 2023-05-14 18:06 ` Mikael Morin
0 siblings, 0 replies; 34+ messages in thread
From: Mikael Morin @ 2023-05-14 18:06 UTC (permalink / raw)
To: Bernhard Reutner-Fischer, Thomas Koenig
Cc: fortran, gcc-patches, Bernhard Reutner-Fischer
Le 14/05/2023 à 17:24, Bernhard Reutner-Fischer a écrit :
> On Sun, 14 May 2023 15:04:15 +0200
> Thomas Koenig <tkoenig@netcologne.de> wrote:
>
>> On 14.05.23 14:27, Mikael Morin wrote:
>>>
>>> (...)
>>>> @@ -2098,7 +2098,7 @@ ref_same_as_full_array (gfc_ref *full_ref,
>>>> gfc_ref *ref)
>>>> there is some kind of overlap.
>>>> 0 : array references are identical or not overlapping. */
>>>> -int
>>>> +bool
>>>> gfc_dep_resolver (gfc_ref *lref, gfc_ref *rref, gfc_reverse *reverse,
>>>> bool identical)
>>>> {
>>>
>>> The function comment states that the function may return 2, which
>>> doesn't seem to be the case any more.
>>
>> Hm, this makes me a litte suspicious. Was functionality for reversing
>> loops lost, maybe unintentionally? I assume that, at some time, we did
>> use the '2' as return value (or did we?)
>
> There was 7c428aa29d75ef163c334cf3974f87b3630d8b8b (a revert because it
> miscompiled spec2k) which might have associated the comment of the
> former static gfc_dependency dep_ref (gfc_ref *lref, gfc_ref *rref,
> gfc_reverse *reverse) to the current gfc_dep_resolver.
>
> The commit which introduced the return value 2 documentation was
> 3d03ead0b8273efde57f6194617b35111a84b05d
> "re PR fortran/24524 (Fortran dependency checking should reverse loops)"
>
> but TBH i don't see how it returned 2 in that revision?
> Looks like when writing that patch it deemed useful to return 2 for
> this specific situation but in the end it was dropped but the comment
> survived.
>
Yes, I came to the same conclusion that we never returned 2 here.
The information that reversal is needed is already provided on a per
dimension basis by the gfc_reverse pointer passed as argument, so
providing the information in the return value would be redundant anyway.
> I will update the comment to document the true / false return values.
>
> And Mikael, do you want me to cleanup 1/0 to true/false assignments for
> the boolean variables, or can we do that in a separate patch (or not at
> all right now)?
>
I don't mind too much either way.
As long as the variables are not assigned integer values outside of the
[0,1] range, and we consistently use true/false or 0/1, not a mix of
them, it's fine with me.
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2] Fortran: Narrow return types [PR78798]
2023-05-14 12:27 ` Mikael Morin
2023-05-14 13:04 ` Thomas Koenig
@ 2023-05-18 19:52 ` Bernhard Reutner-Fischer
1 sibling, 0 replies; 34+ messages in thread
From: Bernhard Reutner-Fischer @ 2023-05-18 19:52 UTC (permalink / raw)
To: Mikael Morin; +Cc: fortran, gcc-patches, Bernhard Reutner-Fischer
On Sun, 14 May 2023 14:27:42 +0200
Mikael Morin <morin-mikael@orange.fr> wrote:
> Le 10/05/2023 à 18:47, Bernhard Reutner-Fischer via Fortran a écrit :
> > From: Bernhard Reutner-Fischer <aldot@gcc.gnu.org>
> >
> > gcc/fortran/ChangeLog:
> >
> > PR fortran/78798
> > * array.cc (compare_bounds): Use narrower return type.
> > (gfc_compare_array_spec): Likewise.
> > (is_constant_element): Likewise.
> > (gfc_constant_ac): Likewise.
> (...)
> > ---
> > Bootstrapped without new warnings and regression tested on
> > x86_64-linux with no regressions, OK for trunk?
> >
> (...)
> > diff --git a/gcc/fortran/check.cc b/gcc/fortran/check.cc
> > index b348bda6e6c..4e3aed84b9d 100644
> > --- a/gcc/fortran/check.cc
> > +++ b/gcc/fortran/check.cc
> > @@ -1156,7 +1156,7 @@ dim_rank_check (gfc_expr *dim, gfc_expr *array, int allow_assumed)
> > dimension bi, returning 0 if they are known not to be identical,
> > and 1 if they are identical, or if this cannot be determined. */
> >
> > -static int
> > +static bool
> > identical_dimen_shape (gfc_expr *a, int ai, gfc_expr *b, int bi)
> > {
> > mpz_t a_size, b_size;
>
> To be consistent, please change as well the local variable "ret" used as
> return value from int to bool.
>
> > diff --git a/gcc/fortran/cpp.cc b/gcc/fortran/cpp.cc
> > index c3b7c7f7bd9..d7890a97287 100644
> > --- a/gcc/fortran/cpp.cc
> > +++ b/gcc/fortran/cpp.cc
> > @@ -297,7 +297,7 @@ gfc_cpp_init_options (unsigned int decoded_options_count,
> > gfc_cpp_option.deferred_opt_count = 0;
> > }
> >
> > -int
> > +bool
> > gfc_cpp_handle_option (size_t scode, const char *arg, int value ATTRIBUTE_UNUSED)
> > {
> > int result = 1;
>
> Same here, change the type of variable "result".
>
> (...)
> > diff --git a/gcc/fortran/dependency.cc b/gcc/fortran/dependency.cc
> > index a648d5c7903..b398b29a642 100644
> > --- a/gcc/fortran/dependency.cc
> > +++ b/gcc/fortran/dependency.cc
> (...)
>
> > @@ -1091,7 +1091,7 @@ gfc_check_argument_dependency (gfc_expr *other, sym_intent intent,
> > /* Like gfc_check_argument_dependency, but check all the arguments in ACTUAL.
> > FNSYM is the function being called, or NULL if not known. */
> >
> > -int
> > +bool
> > gfc_check_fncall_dependency (gfc_expr *other, sym_intent intent,
> > gfc_symbol *fnsym, gfc_actual_arglist *actual,
> > gfc_dep_check elemental)
>
> Why not change the associated subfunctions
> (gfc_check_argument_dependency, gfc_check_argument_var_dependency) as well ?
I have left these subfunctions alone for now to get the other hunks out
of the way. I have adjusted the patch according to your other comments
and pushed it as r14-973-gc072df1ab14450.
Thanks!
>
> (...)
> > @@ -2098,7 +2098,7 @@ ref_same_as_full_array (gfc_ref *full_ref, gfc_ref *ref)
> > there is some kind of overlap.
> > 0 : array references are identical or not overlapping. */
> >
> > -int
> > +bool
> > gfc_dep_resolver (gfc_ref *lref, gfc_ref *rref, gfc_reverse *reverse,
> > bool identical)
> > {
>
> The function comment states that the function may return 2, which
> doesn't seem to be the case any more. So please update the comment.
>
> (...)> diff --git a/gcc/fortran/symbol.cc b/gcc/fortran/symbol.cc
> > index 221165d6dac..b4b36e27d75 100644
> > --- a/gcc/fortran/symbol.cc
> > +++ b/gcc/fortran/symbol.cc
> > @@ -3216,7 +3216,7 @@ gfc_find_symtree_in_proc (const char* name, gfc_namespace* ns)
> > any parent namespaces if requested by a nonzero parent_flag.
> > Returns nonzero if the name is ambiguous. */
> >
> > -int
> > +bool
> > gfc_find_sym_tree (const char *name, gfc_namespace *ns, int parent_flag,
> > gfc_symtree **result)
> > {
>
> Maybe change nonzero to true in the comment?
>
> (...)
>
> OK with all the above fixed.
>
> Thanks.
>
^ permalink raw reply [flat|nested] 34+ messages in thread
end of thread, other threads:[~2023-05-18 19:52 UTC | newest]
Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-12 23:45 [PATCH 0/5] function result decl location; type demotion Bernhard Reutner-Fischer
2022-11-12 23:45 ` [PATCH 1/5] c: Set the locus of the function result decl Bernhard Reutner-Fischer
2022-11-14 21:25 ` Joseph Myers
2022-11-12 23:45 ` [PATCH 2/5] c++: " Bernhard Reutner-Fischer
2022-11-15 23:52 ` Jason Merrill
2022-11-17 8:56 ` Bernhard Reutner-Fischer
2022-11-17 14:53 ` Jason Merrill
2022-11-17 19:02 ` Bernhard Reutner-Fischer
2022-11-17 23:52 ` Jason Merrill
2022-11-18 10:49 ` Bernhard Reutner-Fischer
2022-11-18 16:06 ` Jason Merrill
2022-11-18 18:26 ` Bernhard Reutner-Fischer
2022-11-19 9:56 ` Bernhard Reutner-Fischer
2022-11-20 17:06 ` Bernhard Reutner-Fischer
2022-11-22 20:25 ` Jason Merrill
2022-11-23 15:28 ` Jason Merrill
2022-12-02 19:30 ` Jason Merrill
2022-12-02 19:55 ` Bernhard Reutner-Fischer
2022-11-12 23:45 ` [PATCH 3/5] Fortran: Narrow return types [PR78798] Bernhard Reutner-Fischer
2022-11-13 10:13 ` Janne Blomqvist
2022-11-13 10:39 ` Bernhard Reutner-Fischer
2022-11-13 20:29 ` Harald Anlauf
2022-11-13 20:29 ` Harald Anlauf
2022-11-13 21:50 ` Bernhard Reutner-Fischer
2023-05-10 16:47 ` [PATCH v2] " Bernhard Reutner-Fischer
2023-05-14 12:27 ` Mikael Morin
2023-05-14 13:04 ` Thomas Koenig
2023-05-14 15:24 ` Bernhard Reutner-Fischer
2023-05-14 18:06 ` Mikael Morin
2023-05-18 19:52 ` Bernhard Reutner-Fischer
2022-11-12 23:45 ` [PATCH 4/5] value-range: Add as_string diagnostics helper Bernhard Reutner-Fischer
2022-11-12 23:55 ` Andrew Pinski
2022-11-17 3:30 ` Jeff Law
2022-11-12 23:45 ` [PATCH 5/5] gimple: Add pass to note possible type demotions; IPA pro/demotion; DO NOT MERGE Bernhard Reutner-Fischer
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).