[PATCH v2 3/3] c++: Improve location information in constexpr evaluation
On Fri, Jun 23, 2023 at 01:09:14PM -0400, Patrick Palka wrote: > On Wed, 29 Mar 2023, Nathaniel Shead via Gcc-patches wrote: > > > This patch caches the current expression's location information in the > > constexpr_global_ctx struct, which allows subexpressions that have lost > > location information to still provide accurate diagnostics. Also > > rewrites a number of 'error' calls as 'error_at' to provide more > > specific location information. > > > > The primary effect of this change is that many errors within evaluation > > of a constexpr function will now point at the offending expression (with > > expansion tracing information) rather than just the outermost call. > > This seems like a great improvement! > > In other parts of the frontend, e.g. during substitution from > tsubst_expr or tsubst_copy_and_build, we do something similar by > setting/restoring input_location directly. (We've since added the RAII > class iloc_sentinel for this.) I wonder if that'd be preferable here? I didn't consider that; I've given it a try and I think it's nicer. Doing it this way also updated a number of 'error' calls that I hadn't fixed up in this version; generally this meant nicer error messages, but I had to override it for a couple of cases where I felt the errors it raised were worse (by adding context that made no sense). I'm still bootstrapping/regtesting but I'll send out an updated version of this sometime tomorrow when it's done. Thanks! > > > > gcc/cp/ChangeLog: > > > > * constexpr.cc (constexpr_global_ctx): New field for cached > > tree location, defaulting to input_location. > > (cxx_eval_internal_function): Fall back to ctx->global->loc > > rather than input_location. > > (modifying_const_object_error): Likewise. > > (cxx_eval_dynamic_cast_fn): Likewise. > > (eval_and_check_array_index): Likewise. > > (cxx_eval_array_reference): Likewise. > > (cxx_eval_bit_field_ref): Likewise. > > (cxx_eval_component_reference): Likewise. > > (cxx_eval_indirect_ref): Likewise. > > (cxx_eval_store_expression): Likewise. > > (cxx_eval_increment_expression): Likewise. > > (cxx_eval_loop_expr): Likewise. > > (cxx_eval_binary_expression): Likewise. > > (cxx_eval_constant_expression): Cache location of trees for use > > in errors, and prefer it instead of input_location. > > > > gcc/testsuite/ChangeLog: > > > > * g++.dg/cpp0x/constexpr-48089.C: Updated diagnostic locations. > > * g++.dg/cpp0x/constexpr-diag3.C: Likewise. > > * g++.dg/cpp0x/constexpr-ice20.C: Likewise. > > * g++.dg/cpp1y/constexpr-89481.C: Likewise. > > * g++.dg/cpp1y/constexpr-lifetime1.C: Likewise. > > * g++.dg/cpp1y/constexpr-lifetime2.C: Likewise. > > * g++.dg/cpp1y/constexpr-lifetime3.C: Likewise. > > * g++.dg/cpp1y/constexpr-lifetime4.C: Likewise. > > * g++.dg/cpp1y/constexpr-lifetime5.C: Likewise. > > * g++.dg/cpp1y/constexpr-union5.C: Likewise. > > * g++.dg/cpp1y/pr68180.C: Likewise. > > * g++.dg/cpp1z/constexpr-lambda6.C: Likewise. > > * g++.dg/cpp2a/bit-cast11.C: Likewise. > > * g++.dg/cpp2a/bit-cast12.C: Likewise. > > * g++.dg/cpp2a/bit-cast14.C: Likewise. > > * g++.dg/cpp2a/constexpr-98122.C: Likewise. > > * g++.dg/cpp2a/constexpr-dynamic17.C: Likewise. > > * g++.dg/cpp2a/constexpr-init1.C: Likewise. > > * g++.dg/cpp2a/constexpr-new12.C: Likewise. > > * g++.dg/cpp2a/constexpr-new3.C: Likewise. > > * g++.dg/ext/constexpr-vla2.C: Likewise. > > * g++.dg/ext/constexpr-vla3.C: Likewise. > > * g++.dg/ubsan/pr63956.C: Likewise. > > > > libstdc++/ChangeLog: > > > > * testsuite/25_algorithms/equal/constexpr_neg.cc: Updated > > diagnostics locations. > > > > Signed-off-by: Nathaniel Shead > > --- > > gcc/cp/constexpr.cc | 83 +++ > > gcc/testsuite/g++.dg/cpp0x/constexpr-48089.C | 10 +-- > > gcc/testsuite/g++.dg/cpp0x/constexpr-diag3.C | 2 +- > > gcc/testsuite/g++.dg/cpp0x/constexpr-ice20.C | 4 +- > > gcc/testsuite/g++.dg/cpp1y/constexpr-89481.C | 3 +- > > .../g++.dg/cpp1y/constexpr-lifetime1.C| 1 + > > .../g++.dg/cpp1y/constexpr-lifetime2.C| 4 +- > > .../g++.dg/cpp1y/constexpr-lifetime3.C| 4 +- > > .../g++.dg/cpp1y/constexpr-lifetime4.C| 2 +- > > .../g++.dg/cpp1y/constexpr-lifetime5.C| 4 +- > > gcc/testsuite/g++.dg/cpp1y/constexpr-union5.C | 4 +- > > gcc/testsuite/g++.dg/cpp1y/pr68180.C | 4 +- > > .../g++.dg/cpp1z/constexpr-lambda6.C | 4 +- > > gcc/testsuite/g++.dg/cpp2a/bit-cast11.C | 10 +-- > > gcc/testsuite/g++.dg/cpp2a/bit-cast12.C | 10 +-- > > gcc/testsuite/g++.dg/cpp2a/bit-cast14.C | 14 ++-- > > gcc/testsuite/g++.dg/cpp2a/constexpr-98122.C | 4 +- > > .../g++.dg/cpp2a/constexpr-dynamic17.C| 5 +- > > gcc/testsuite/g++.dg/cpp2a/constexpr-init1.C | 5 +- > > gcc/testsuite/g++.dg/cpp2a/constexpr-new12.C | 6
Re: [PATCH v2 3/3] c++: Improve location information in constexpr evaluation
On Wed, 29 Mar 2023, Nathaniel Shead via Gcc-patches wrote: > This patch caches the current expression's location information in the > constexpr_global_ctx struct, which allows subexpressions that have lost > location information to still provide accurate diagnostics. Also > rewrites a number of 'error' calls as 'error_at' to provide more > specific location information. > > The primary effect of this change is that many errors within evaluation > of a constexpr function will now point at the offending expression (with > expansion tracing information) rather than just the outermost call. This seems like a great improvement! In other parts of the frontend, e.g. during substitution from tsubst_expr or tsubst_copy_and_build, we do something similar by setting/restoring input_location directly. (We've since added the RAII class iloc_sentinel for this.) I wonder if that'd be preferable here? > > gcc/cp/ChangeLog: > > * constexpr.cc (constexpr_global_ctx): New field for cached > tree location, defaulting to input_location. > (cxx_eval_internal_function): Fall back to ctx->global->loc > rather than input_location. > (modifying_const_object_error): Likewise. > (cxx_eval_dynamic_cast_fn): Likewise. > (eval_and_check_array_index): Likewise. > (cxx_eval_array_reference): Likewise. > (cxx_eval_bit_field_ref): Likewise. > (cxx_eval_component_reference): Likewise. > (cxx_eval_indirect_ref): Likewise. > (cxx_eval_store_expression): Likewise. > (cxx_eval_increment_expression): Likewise. > (cxx_eval_loop_expr): Likewise. > (cxx_eval_binary_expression): Likewise. > (cxx_eval_constant_expression): Cache location of trees for use > in errors, and prefer it instead of input_location. > > gcc/testsuite/ChangeLog: > > * g++.dg/cpp0x/constexpr-48089.C: Updated diagnostic locations. > * g++.dg/cpp0x/constexpr-diag3.C: Likewise. > * g++.dg/cpp0x/constexpr-ice20.C: Likewise. > * g++.dg/cpp1y/constexpr-89481.C: Likewise. > * g++.dg/cpp1y/constexpr-lifetime1.C: Likewise. > * g++.dg/cpp1y/constexpr-lifetime2.C: Likewise. > * g++.dg/cpp1y/constexpr-lifetime3.C: Likewise. > * g++.dg/cpp1y/constexpr-lifetime4.C: Likewise. > * g++.dg/cpp1y/constexpr-lifetime5.C: Likewise. > * g++.dg/cpp1y/constexpr-union5.C: Likewise. > * g++.dg/cpp1y/pr68180.C: Likewise. > * g++.dg/cpp1z/constexpr-lambda6.C: Likewise. > * g++.dg/cpp2a/bit-cast11.C: Likewise. > * g++.dg/cpp2a/bit-cast12.C: Likewise. > * g++.dg/cpp2a/bit-cast14.C: Likewise. > * g++.dg/cpp2a/constexpr-98122.C: Likewise. > * g++.dg/cpp2a/constexpr-dynamic17.C: Likewise. > * g++.dg/cpp2a/constexpr-init1.C: Likewise. > * g++.dg/cpp2a/constexpr-new12.C: Likewise. > * g++.dg/cpp2a/constexpr-new3.C: Likewise. > * g++.dg/ext/constexpr-vla2.C: Likewise. > * g++.dg/ext/constexpr-vla3.C: Likewise. > * g++.dg/ubsan/pr63956.C: Likewise. > > libstdc++/ChangeLog: > > * testsuite/25_algorithms/equal/constexpr_neg.cc: Updated > diagnostics locations. > > Signed-off-by: Nathaniel Shead > --- > gcc/cp/constexpr.cc | 83 +++ > gcc/testsuite/g++.dg/cpp0x/constexpr-48089.C | 10 +-- > gcc/testsuite/g++.dg/cpp0x/constexpr-diag3.C | 2 +- > gcc/testsuite/g++.dg/cpp0x/constexpr-ice20.C | 4 +- > gcc/testsuite/g++.dg/cpp1y/constexpr-89481.C | 3 +- > .../g++.dg/cpp1y/constexpr-lifetime1.C| 1 + > .../g++.dg/cpp1y/constexpr-lifetime2.C| 4 +- > .../g++.dg/cpp1y/constexpr-lifetime3.C| 4 +- > .../g++.dg/cpp1y/constexpr-lifetime4.C| 2 +- > .../g++.dg/cpp1y/constexpr-lifetime5.C| 4 +- > gcc/testsuite/g++.dg/cpp1y/constexpr-union5.C | 4 +- > gcc/testsuite/g++.dg/cpp1y/pr68180.C | 4 +- > .../g++.dg/cpp1z/constexpr-lambda6.C | 4 +- > gcc/testsuite/g++.dg/cpp2a/bit-cast11.C | 10 +-- > gcc/testsuite/g++.dg/cpp2a/bit-cast12.C | 10 +-- > gcc/testsuite/g++.dg/cpp2a/bit-cast14.C | 14 ++-- > gcc/testsuite/g++.dg/cpp2a/constexpr-98122.C | 4 +- > .../g++.dg/cpp2a/constexpr-dynamic17.C| 5 +- > gcc/testsuite/g++.dg/cpp2a/constexpr-init1.C | 5 +- > gcc/testsuite/g++.dg/cpp2a/constexpr-new12.C | 6 +- > gcc/testsuite/g++.dg/cpp2a/constexpr-new3.C | 10 +-- > gcc/testsuite/g++.dg/ext/constexpr-vla2.C | 4 +- > gcc/testsuite/g++.dg/ext/constexpr-vla3.C | 4 +- > gcc/testsuite/g++.dg/ubsan/pr63956.C | 4 +- > .../25_algorithms/equal/constexpr_neg.cc | 7 +- > 25 files changed, 111 insertions(+), 101 deletions(-) > > diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc > index bdbc12144a7..74045477a92 100644 > --- a/gcc/cp/constexpr.cc > +++ b/gcc/cp/constexpr.cc > @@ -1165,10 +1165,12 @@ public: >hash_set *modifiable; >/* Number of heap VAR_DECL deallocations. */ >unsigned h
[PATCH v2 3/3] c++: Improve location information in constexpr evaluation
This patch caches the current expression's location information in the
constexpr_global_ctx struct, which allows subexpressions that have lost
location information to still provide accurate diagnostics. Also
rewrites a number of 'error' calls as 'error_at' to provide more
specific location information.
The primary effect of this change is that many errors within evaluation
of a constexpr function will now point at the offending expression (with
expansion tracing information) rather than just the outermost call.
gcc/cp/ChangeLog:
* constexpr.cc (constexpr_global_ctx): New field for cached
tree location, defaulting to input_location.
(cxx_eval_internal_function): Fall back to ctx->global->loc
rather than input_location.
(modifying_const_object_error): Likewise.
(cxx_eval_dynamic_cast_fn): Likewise.
(eval_and_check_array_index): Likewise.
(cxx_eval_array_reference): Likewise.
(cxx_eval_bit_field_ref): Likewise.
(cxx_eval_component_reference): Likewise.
(cxx_eval_indirect_ref): Likewise.
(cxx_eval_store_expression): Likewise.
(cxx_eval_increment_expression): Likewise.
(cxx_eval_loop_expr): Likewise.
(cxx_eval_binary_expression): Likewise.
(cxx_eval_constant_expression): Cache location of trees for use
in errors, and prefer it instead of input_location.
gcc/testsuite/ChangeLog:
* g++.dg/cpp0x/constexpr-48089.C: Updated diagnostic locations.
* g++.dg/cpp0x/constexpr-diag3.C: Likewise.
* g++.dg/cpp0x/constexpr-ice20.C: Likewise.
* g++.dg/cpp1y/constexpr-89481.C: Likewise.
* g++.dg/cpp1y/constexpr-lifetime1.C: Likewise.
* g++.dg/cpp1y/constexpr-lifetime2.C: Likewise.
* g++.dg/cpp1y/constexpr-lifetime3.C: Likewise.
* g++.dg/cpp1y/constexpr-lifetime4.C: Likewise.
* g++.dg/cpp1y/constexpr-lifetime5.C: Likewise.
* g++.dg/cpp1y/constexpr-union5.C: Likewise.
* g++.dg/cpp1y/pr68180.C: Likewise.
* g++.dg/cpp1z/constexpr-lambda6.C: Likewise.
* g++.dg/cpp2a/bit-cast11.C: Likewise.
* g++.dg/cpp2a/bit-cast12.C: Likewise.
* g++.dg/cpp2a/bit-cast14.C: Likewise.
* g++.dg/cpp2a/constexpr-98122.C: Likewise.
* g++.dg/cpp2a/constexpr-dynamic17.C: Likewise.
* g++.dg/cpp2a/constexpr-init1.C: Likewise.
* g++.dg/cpp2a/constexpr-new12.C: Likewise.
* g++.dg/cpp2a/constexpr-new3.C: Likewise.
* g++.dg/ext/constexpr-vla2.C: Likewise.
* g++.dg/ext/constexpr-vla3.C: Likewise.
* g++.dg/ubsan/pr63956.C: Likewise.
libstdc++/ChangeLog:
* testsuite/25_algorithms/equal/constexpr_neg.cc: Updated
diagnostics locations.
Signed-off-by: Nathaniel Shead
---
gcc/cp/constexpr.cc | 83 +++
gcc/testsuite/g++.dg/cpp0x/constexpr-48089.C | 10 +--
gcc/testsuite/g++.dg/cpp0x/constexpr-diag3.C | 2 +-
gcc/testsuite/g++.dg/cpp0x/constexpr-ice20.C | 4 +-
gcc/testsuite/g++.dg/cpp1y/constexpr-89481.C | 3 +-
.../g++.dg/cpp1y/constexpr-lifetime1.C| 1 +
.../g++.dg/cpp1y/constexpr-lifetime2.C| 4 +-
.../g++.dg/cpp1y/constexpr-lifetime3.C| 4 +-
.../g++.dg/cpp1y/constexpr-lifetime4.C| 2 +-
.../g++.dg/cpp1y/constexpr-lifetime5.C| 4 +-
gcc/testsuite/g++.dg/cpp1y/constexpr-union5.C | 4 +-
gcc/testsuite/g++.dg/cpp1y/pr68180.C | 4 +-
.../g++.dg/cpp1z/constexpr-lambda6.C | 4 +-
gcc/testsuite/g++.dg/cpp2a/bit-cast11.C | 10 +--
gcc/testsuite/g++.dg/cpp2a/bit-cast12.C | 10 +--
gcc/testsuite/g++.dg/cpp2a/bit-cast14.C | 14 ++--
gcc/testsuite/g++.dg/cpp2a/constexpr-98122.C | 4 +-
.../g++.dg/cpp2a/constexpr-dynamic17.C| 5 +-
gcc/testsuite/g++.dg/cpp2a/constexpr-init1.C | 5 +-
gcc/testsuite/g++.dg/cpp2a/constexpr-new12.C | 6 +-
gcc/testsuite/g++.dg/cpp2a/constexpr-new3.C | 10 +--
gcc/testsuite/g++.dg/ext/constexpr-vla2.C | 4 +-
gcc/testsuite/g++.dg/ext/constexpr-vla3.C | 4 +-
gcc/testsuite/g++.dg/ubsan/pr63956.C | 4 +-
.../25_algorithms/equal/constexpr_neg.cc | 7 +-
25 files changed, 111 insertions(+), 101 deletions(-)
diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
index bdbc12144a7..74045477a92 100644
--- a/gcc/cp/constexpr.cc
+++ b/gcc/cp/constexpr.cc
@@ -1165,10 +1165,12 @@ public:
hash_set *modifiable;
/* Number of heap VAR_DECL deallocations. */
unsigned heap_dealloc_count;
+ /* Current location in case subtree has no location information. */
+ location_t loc;
/* Constructor. */
constexpr_global_ctx ()
: constexpr_ops_count (0), cleanups (NULL), modifiable (nullptr),
- heap_dealloc_count (0) {}
+ heap_dealloc_count (0), loc (input_location) {}
tree get_value (tree t)
{
@@ -2113,7 +2115,7 @@ cxx_eval_internal_function (const constexpr_ctx *ctx,
tree t,
default:
if (!ctx->quiet)
-
