Re: [PATCH 2/5] c++: Set the locus of the function result decl
On 2 December 2022 20:30:56 CET, Jason Merrill 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,
Re: [PATCH 2/5] c++: Set the locus of the function result decl
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:
From d19aa6af6634b1e97f38431ad091f3b3f12baf2f Mon Sep 17 00:00:00 2001
From: Bernhard Reutner-Fischer
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
---
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
Re: [PATCH 2/5] c++: Set the locus of the function result decl
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:
From 3c8106c95ec07d17ff5ade173126067c540ab7cc Mon Sep 17 00:00:00 2001
From: Bernhard Reutner-Fischer
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
---
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
@@
Re: [PATCH 2/5] c++: Set the locus of the function result decl
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:
From 5075d2ac12f655f8f83f6f3be27e2c1141e1ce99 Mon Sep 17 00:00:00 2001
From: Bernhard Reutner-Fischer
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
---
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
Re: [PATCH 2/5] c++: Set the locus of the function result decl
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; } /* 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)) +:
Re: [PATCH 2/5] c++: Set the locus of the function result decl
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 000..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 000..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
+int
+f()
+{
+ return 42;
+}
+int main()
+{
+ f();
+}
+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
+-int
++bool
+ f()
+ {
+ return 42;
+ }
+-int main()
++bool main()
+ {
+f();
+ }
+-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 000..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" } */
Re: [PATCH 2/5] c++: Set the locus of the function result decl
On Fri, 18 Nov 2022 11:06:29 -0500
Jason Merrill 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.
Re: [PATCH 2/5] c++: Set the locus of the function result decl
On 11/18/22 05:49, Bernhard Reutner-Fischer wrote:
On Thu, 17 Nov 2022 18:52:36 -0500
Jason Merrill wrote:
On 11/17/22 14:02, Bernhard Reutner-Fischer wrote:
On Thu, 17 Nov 2022 09:53:32 -0500
Jason Merrill 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 int f() { return 42; }
int main()
{
f();
}
#1 0x00f950e8 in instantiate_body (pattern=, args=, d=, nested_p=false) at /home/jason/gt/gcc/cp/pt.cc:26470
#0 start_preparsed_function (decl1=,
attrs=, 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 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 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<
>::_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<
>::_M_space_left() [with =
std::pair]’:
../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<
>::_M_space_left() [with = 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
Re: [PATCH 2/5] c++: Set the locus of the function result decl
On Thu, 17 Nov 2022 18:52:36 -0500
Jason Merrill wrote:
> On 11/17/22 14:02, Bernhard Reutner-Fischer wrote:
> > On Thu, 17 Nov 2022 09:53:32 -0500
> > Jason Merrill 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 int f() { return 42; }
>
> int main()
> {
>f();
> }
>
> #1 0x00f950e8 in instantiate_body (pattern= 0x77ff5080 f>, args=, d= 0x7fffe971e600 f>, nested_p=false) at /home/jason/gt/gcc/cp/pt.cc:26470
> #0 start_preparsed_function (decl1=,
> attrs=, 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 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 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< >::_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< >::_M_space_left() [with
= std::pair]’:
../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< >::_M_space_left() [with
= 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,
diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
index
Re: [PATCH 2/5] c++: Set the locus of the function result decl
On 11/17/22 14:02, Bernhard Reutner-Fischer wrote:
On Thu, 17 Nov 2022 09:53:32 -0500
Jason Merrill wrote:
On 11/17/22 03:56, Bernhard Reutner-Fischer wrote:
On Tue, 15 Nov 2022 18:52:41 -0500
Jason Merrill 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
Cc: Jason Merrill
---
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, );
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 int f() { return 42; }
int main()
{
f();
}
#1 0x00f950e8 in instantiate_body (pattern=0x77ff5080 f>, args=, d=0x7fffe971e600 f>, nested_p=false) at /home/jason/gt/gcc/cp/pt.cc:26470
#0 start_preparsed_function (decl1=,
attrs=, 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.
Re: [PATCH 2/5] c++: Set the locus of the function result decl
On Thu, 17 Nov 2022 09:53:32 -0500
Jason Merrill wrote:
> On 11/17/22 03:56, Bernhard Reutner-Fischer wrote:
> > On Tue, 15 Nov 2022 18:52:41 -0500
> > Jason Merrill 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
> >>> Cc: Jason Merrill
> >>> ---
> >>>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, );
> >>> 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 =
Re: [PATCH 2/5] c++: Set the locus of the function result decl
On 11/17/22 03:56, Bernhard Reutner-Fischer wrote:
On Tue, 15 Nov 2022 18:52:41 -0500
Jason Merrill 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
Cc: Jason Merrill
---
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, );
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;
}
/* Returns true iff an EH_SPEC_BLOCK should be created in the body of
Re: [PATCH 2/5] c++: Set the locus of the function result decl
On Tue, 15 Nov 2022 18:52:41 -0500
Jason Merrill 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
> > Cc: Jason Merrill
> > ---
> > 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, );
> > 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;
> > }
> >
> > /* Returns true iff an EH_SPEC_BLOCK should be created in the body of
>
>From 5595eb2d3056f6bca3d2b80bc8b2796d86da0ce8 Mon Sep 17 00:00:00 2001
From: Bernhard Reutner-Fischer
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,
Re: [PATCH 2/5] c++: Set the locus of the function result decl
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
Cc: Jason Merrill
---
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, );
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;
}
/* Returns true iff an EH_SPEC_BLOCK should be created in the body of
