On 2/3/23 13:08, Marek Polacek wrote:
On Thu, Feb 02, 2023 at 05:29:44PM -0500, Jason Merrill wrote:
On 1/30/23 21:35, Marek Polacek wrote:
In this test case, we find ourselves evaluating 't' which is
((const struct carray *) this)->data_[VIEW_CONVERT_EXPR<long int>(index)]
in cxx_eval_array_reference. ctx->object is non-null, a RESULT_DECL, so
we replace it with 't':
new_ctx.object = t; // result_decl replaced
and then we go to cxx_eval_constant_expression to evaluate an
AGGR_INIT_EXPR, where we end up evaluating an INIT_EXPR (which is in the
body of the constructor for seed_or_index):
((struct seed_or_index *) this)->value_ = NON_LVALUE_EXPR <0>
whereupon in cxx_eval_store_expression we go to the probe loop
where the 'this' is evaluated to
ze_set.tables_.first_table_.data_[0]
so the 'object' is ze_set, but that isn't in ctx->global->get_value_ptr
so we fail with a bogus error. ze_set is not there because it comes
from a different constexpr context (it's not in cv_cache either).
The problem started with r12-2304 where I added the new_ctx.object
replacement. That was to prevent a type mismatch: the type of 't'
and ctx.object were different.
It seems clear that we shouldn't have replaced ctx.object here.
The cxx_eval_array_reference I mentioned earlier is called from
cxx_eval_store_expression:
6257 init = cxx_eval_constant_expression (&new_ctx, init, vc_prvalue,
6258 non_constant_p, overflow_p);
which already created a new context, whose .object we should be
using unless, for instance, INIT contained a.b and we're evaluating
the 'a' part, which I think was the case for r12-2304; in that case
ctx.object has to be something different.
A relatively safe fix should be to check the types before replacing
ctx.object, as in the below.
Agreed. I'm trying to understand when the replacement could ever make
sense, since 't' is not the target, it's the initializer. The replacement
comes from Patrick's fix for 98295, but that testcase no longer hits that
code (likely due to changes in empty class handling).
If you add a gcc_checking_assert (false) to the replacement, does anything
trip it?
It would trip in constexpr-101371.C, added in r12-2304. BUT, and I would
have sworn that it ICEd when I tried, it's not necessary anymore. So it
looks like we can simply remove the new_ctx.object line. At least for
trunk, maybe 12 too.
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
OK, thanks. Let's go with your original patch for 11/12.
-- >8 --
In this test case, we find ourselves evaluating 't' which is
((const struct carray *) this)->data_[VIEW_CONVERT_EXPR<long int>(index)]
in cxx_eval_array_reference. ctx->object is non-null, a RESULT_DECL, so
we replace it with 't':
new_ctx.object = t; // result_decl replaced
and then we go to cxx_eval_constant_expression to evaluate an
AGGR_INIT_EXPR, where we end up evaluating an INIT_EXPR (which is in the
body of the constructor for seed_or_index):
((struct seed_or_index *) this)->value_ = NON_LVALUE_EXPR <0>
whereupon in cxx_eval_store_expression we go to the probe loop
where the 'this' is evaluated to
ze_set.tables_.first_table_.data_[0]
so the 'object' is ze_set, but that isn't in ctx->global->get_value_ptr
so we fail with a bogus error. ze_set is not there because it comes
from a different constexpr context (it's not in cv_cache either).
The problem started with r12-2304 where I added the new_ctx.object
replacement. That was to prevent a type mismatch: the type of 't'
and ctx.object were different.
It seems clear that we shouldn't have replaced ctx.object here.
The cxx_eval_array_reference I mentioned earlier is called from
cxx_eval_store_expression:
6257 init = cxx_eval_constant_expression (&new_ctx, init, vc_prvalue,
6258 non_constant_p, overflow_p);
which already created a new context, whose .object we should be
using unless, for instance, INIT contained a.b and we're evaluating
the 'a' part, which I think was the case for r12-2304; in that case
ctx.object has to be something different.
It no longer seems necessary to replace new_ctx.object (likely due to
changes in empty class handling).
PR c++/108158
gcc/cp/ChangeLog:
* constexpr.cc (cxx_eval_array_reference): Don't replace
new_ctx.object.
gcc/testsuite/ChangeLog:
* g++.dg/cpp1y/constexpr-108158.C: New test.
---
gcc/cp/constexpr.cc | 4 ---
gcc/testsuite/g++.dg/cpp1y/constexpr-108158.C | 32 +++++++++++++++++++
2 files changed, 32 insertions(+), 4 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-108158.C
diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
index 5b31f9c27d1..564766c8a00 100644
--- a/gcc/cp/constexpr.cc
+++ b/gcc/cp/constexpr.cc
@@ -4301,10 +4301,6 @@ cxx_eval_array_reference (const constexpr_ctx *ctx, tree
t,
if (!SCALAR_TYPE_P (elem_type))
{
new_ctx = *ctx;
- if (ctx->object)
- /* If there was no object, don't add one: it could confuse us
- into thinking we're modifying a const object. */
- new_ctx.object = t;
new_ctx.ctor = build_constructor (elem_type, NULL);
ctx = &new_ctx;
}
diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-108158.C
b/gcc/testsuite/g++.dg/cpp1y/constexpr-108158.C
new file mode 100644
index 00000000000..e5f5e9954e5
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-108158.C
@@ -0,0 +1,32 @@
+// PR c++/108158
+// { dg-do compile { target c++14 } }
+
+template <class T, int N> struct carray {
+ T data_[N]{};
+ constexpr T operator[](long index) const { return data_[index]; }
+};
+struct seed_or_index {
+private:
+ long value_ = 0;
+};
+template <int M> struct pmh_tables {
+ carray<seed_or_index, M> first_table_;
+ template <typename KeyType, typename HasherType>
+ constexpr void lookup(KeyType, HasherType) const {
+ first_table_[0];
+ }
+};
+template <int N> struct unordered_set {
+ int equal_;
+ carray<int, N> keys_;
+ pmh_tables<N> tables_;
+ constexpr unordered_set() : equal_{} {}
+ template <class KeyType, class Hasher>
+ constexpr auto lookup(KeyType key, Hasher hash) const {
+ tables_.lookup(key, hash);
+ return keys_;
+ }
+};
+constexpr unordered_set<3> ze_set;
+constexpr auto nocount = ze_set.lookup(4, int());
+constexpr auto nocount2 = unordered_set<3>{}.lookup(4, int());
base-commit: c9aef107ce697f58a34734d82f8d2514405c9be0
