In this testcase we have a constexpr function, value_to_char_helper. Its body
is
for (size_t i = 0u; i < alphabet_t::value_size; ++i)
value_to_char[i] = to_char(alphabet.assign_rank(i));
which is genericized to a LOOP_EXPR looking roughly like this:
while (1)
{
if (i > 3)
goto out;
value_to_char[i] = to_char (..., i, ...);
i++;
}
Then this is what happens when evaluating the above:
1) cxx_eval_call_expression evaluates the first call to to_char
it's not in the hash table -> copy the body and the bindings and
then evaluate the body
the result is 65
bindings: alph -> {._value=0} (correct)
save all this to the table
2) cxx_eval_store_expression evaluates i++
then it replaces the value in the hash map:
*valp = init;
which is generally what we want, but that also overwrites {._value=0} from
above to {._value=1} which causes problem in 3)
3) cxx_eval_call_expression tries to evaluate the second call to to_char
bindings: alph -> {._value=1} (correct)
here if the hashes match (use [1] hunk for better testing) then
we find to_char call in the table. Then we invoke
constexpr_call_hasher::equal() to compare the two calls: fundefs
match, and because 2) has overwritten bindings of the previous
to_char call, they match too, both are {._value=1}.
That means we don't evaluate this second call and just use the cached
result (65), and that is wrong.
I think the fix is to avoid sharing the constructor in the bindings which
is what the patch does and what we do in several places already. I think
Jakub's patch <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84684#c13>
wouldn't work if the bindings had more constructors.
But I'm also wondering about massage_init_elt. It has
tree t = fold_non_dependent_expr (init);
t = maybe_constant_init (t);
but given that fold_non_dependent_expr now calls maybe_constant_value, which
then causes that we try to cache the calls above, this seems excessive,
wouldn't we be better off with just calling fold_non_dependent_init as
discussed recently?
I bootstrapped/regtested this patch with this hunk, too, for better testing:
[1]
@@ -1598,8 +1598,12 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree
t,
constexpr_call *entry = NULL;
if (depth_ok && !non_constant_args && ctx->strict)
{
+#if 0
new_call.hash = iterative_hash_template_arg
(new_call.bindings, constexpr_fundef_hasher::hash (new_call.fundef));
+#else
+ new_call.hash = 0;
+#endif
/* If we have seen this call before, we are done. */
maybe_initialize_constexpr_call_table ();
Bootstrapped/regtested on x86_64-linux, ok for trunk? And likely 7.
2018-03-06 Marek Polacek <pola...@redhat.com>
PR c++/84684
* constexpr.c (cxx_bind_parameters_in_call): Unshare evaluated
arguments.
* g++.dg/cpp1z/constexpr-84684.C: New test.
diff --git gcc/cp/constexpr.c gcc/cp/constexpr.c
index 941562ebb05..15a950187ea 100644
--- gcc/cp/constexpr.c
+++ gcc/cp/constexpr.c
@@ -1313,6 +1313,8 @@ cxx_bind_parameters_in_call (const constexpr_ctx *ctx,
tree t,
if (!*non_constant_p)
{
+ /* Don't share a CONSTRUCTOR that might be changed. */
+ arg = unshare_constructor (arg);
/* Make sure the binding has the same type as the parm. But
only for constant args. */
if (TREE_CODE (type) != REFERENCE_TYPE)
diff --git gcc/testsuite/g++.dg/cpp1z/constexpr-84684.C
gcc/testsuite/g++.dg/cpp1z/constexpr-84684.C
index e69de29bb2d..0e7912d4067 100644
--- gcc/testsuite/g++.dg/cpp1z/constexpr-84684.C
+++ gcc/testsuite/g++.dg/cpp1z/constexpr-84684.C
@@ -0,0 +1,163 @@
+// PR c++/84684
+// { dg-options -std=c++17 }
+
+typedef decltype (sizeof (0)) size_t;
+
+namespace std {
+ template<class _E>
+ struct initializer_list
+ {
+ typedef _E value_type;
+ typedef const _E& reference;
+ typedef const _E& const_reference;
+ typedef size_t size_type;
+ typedef const _E* iterator;
+ typedef const _E* const_iterator;
+ iterator _M_array;
+ size_type _M_len;
+ constexpr initializer_list(const_iterator __a, size_type __l) :
_M_array(__a), _M_len(__l) { }
+ constexpr initializer_list() noexcept : _M_array(0), _M_len(0) { }
+ constexpr size_type size() const noexcept { return _M_len; }
+ constexpr const_iterator begin() const noexcept { return _M_array; }
+ constexpr const_iterator end() const noexcept { return begin() + size(); }
+ };
+}
+
+template <typename E, size_t N>
+struct array
+{
+ constexpr E &operator[](size_t n) noexcept { return elems[n]; }
+ constexpr const E &operator[](size_t n) const noexcept { return elems[n]; }
+ constexpr size_t size() const { return N; }
+ E elems[N];
+};
+
+template<typename T>
+constexpr
+inline T
+max (std::initializer_list<T> i)
+{
+ const T *b = i.begin ();
+ const T *e = i.end ();
+ if (b == e) return *b;
+ const T *r = b;
+ while (++b != e)
+ if (*r < *b)
+ r = b;
+ return *r;
+}
+
+template <typename alphabet_type>
+constexpr char to_char(alphabet_type const alph)
+{
+ return alph.to_char();
+}
+
+template <typename ...alphabet_types>
+struct union_composition
+{
+ static constexpr size_t value_size = (alphabet_types::value_size + ... );
+ unsigned char _value;
+ template <size_t fixed_size, typename alphabet_t>
+ static constexpr auto value_to_char_helper(alphabet_t alphabet)
+ {
+ array<char, fixed_size> value_to_char{};
+ for (size_t i = 0u; i < alphabet_t::value_size; ++i)
+ value_to_char[i] = to_char(alphabet.assign_rank(i));
+ return value_to_char;
+ }
+
+ static constexpr auto make_value_to_char()
+ {
+ constexpr auto N = sizeof...(alphabet_types);
+ constexpr array<size_t, N> alphabet_sizes { alphabet_types::value_size...
};
+ constexpr size_t fixed_size = max({alphabet_types::value_size...});
+ array value_to_char_tables = array<array<char, fixed_size>, N> {
+ value_to_char_helper<fixed_size>(alphabet_types{})...
+ };
+ array<char, value_size> value_to_char{};
+ for (size_t i = 0u, value = 0u; i < N; ++i)
+ for (size_t k = 0u; k < alphabet_sizes[i]; ++k, ++value)
+ value_to_char[value] = value_to_char_tables[i][k];
+ return value_to_char;
+ }
+};
+
+struct gap
+{
+ constexpr char to_char() const noexcept { return '-'; }
+ constexpr gap & assign_rank([[maybe_unused]] bool const i) noexcept { return
*this; }
+ static constexpr size_t value_size{1};
+};
+
+struct dna4
+{
+ constexpr char to_char() const noexcept { return value_to_char[_value]; }
+ constexpr dna4 & assign_rank(unsigned char const c) { _value = c; return
*this; }
+ static constexpr size_t value_size{4};
+ static constexpr char value_to_char[value_size] { 'A', 'C', 'G', 'T' };
+ unsigned char _value;
+};
+
+struct dna5
+{
+ constexpr char to_char() const noexcept { return value_to_char[_value]; }
+ constexpr dna5 & assign_rank(unsigned char const c) { _value = c; return
*this; }
+ static constexpr size_t value_size{5};
+ static constexpr char value_to_char[value_size] { 'A', 'C', 'G', 'T', 'N' };
+ unsigned char _value;
+};
+
+constexpr array value_to_char1 = union_composition<dna4>::make_value_to_char();
+static_assert(value_to_char1.size() == 4u);
+static_assert(value_to_char1[0] == 'A');
+static_assert(value_to_char1[1] == 'C');
+static_assert(value_to_char1[2] == 'G');
+static_assert(value_to_char1[3] == 'T');
+
+constexpr array value_to_char2 = union_composition<dna4,
gap>::make_value_to_char();
+static_assert(value_to_char2.size() == 5u);
+static_assert(value_to_char2[0] == 'A');
+static_assert(value_to_char2[1] == 'C');
+static_assert(value_to_char2[2] == 'G');
+static_assert(value_to_char2[3] == 'T');
+static_assert(value_to_char2[4] == '-');
+
+constexpr array value_to_char3 = union_composition<dna4, gap,
dna5>::make_value_to_char();
+static_assert(value_to_char3.size() == 10u);
+static_assert(value_to_char3[0] == 'A');
+static_assert(value_to_char3[1] == 'C');
+static_assert(value_to_char3[2] == 'G');
+static_assert(value_to_char3[3] == 'T');
+static_assert(value_to_char3[4] == '-');
+static_assert(value_to_char3[5] == 'A');
+static_assert(value_to_char3[6] == 'C');
+static_assert(value_to_char3[7] == 'G');
+static_assert(value_to_char3[8] == 'T');
+static_assert(value_to_char3[9] == 'N');
+
+constexpr array value_to_char4 = union_composition<dna5, gap,
dna4>::make_value_to_char();
+static_assert(value_to_char4.size() == 10u);
+static_assert(value_to_char4[0] == 'A');
+static_assert(value_to_char4[1] == 'C');
+static_assert(value_to_char4[2] == 'G');
+static_assert(value_to_char4[3] == 'T');
+static_assert(value_to_char4[4] == 'N');
+static_assert(value_to_char4[5] == '-');
+static_assert(value_to_char4[6] == 'A');
+static_assert(value_to_char4[7] == 'C');
+static_assert(value_to_char4[8] == 'G');
+static_assert(value_to_char4[9] == 'T');
+
+constexpr array value_to_char5 = union_composition<gap, dna4,
dna5>::make_value_to_char();
+static_assert(value_to_char5.size() == 10u);
+static_assert(value_to_char5[0] == '-');
+static_assert(value_to_char5[1] == 'A');
+static_assert(value_to_char5[2] == 'C');
+static_assert(value_to_char5[3] == 'G');
+static_assert(value_to_char5[4] == 'T');
+static_assert(value_to_char5[5] == 'A');
+static_assert(value_to_char5[6] == 'C');
+static_assert(value_to_char5[7] == 'G');
+static_assert(value_to_char5[8] == 'T');
+static_assert(value_to_char5[9] == 'N');
Marek
