2015-07-27 18:51 GMT+02:00 Jason Merrill <ja...@redhat.com>:
> I've trimmed this to the previously mentioned issues that still need to be
> addressed; I'll do another full review after these are dealt with.
Thanks for doing this summary of missing parts of prior review.
> On 06/13/2015 12:15 AM, Jason Merrill wrote:
>>
>> On 06/12/2015 12:11 PM, Kai Tietz wrote:
>>>>>
>>>>> @@ -1052,6 +1054,9 @@ adjust_temp_type (tree type, tree temp)
>>>>> {
>>>>> if (TREE_TYPE (temp) == type)
>>>>> return temp;
>>>>> + STRIP_NOPS (temp);
>>>>> + if (TREE_TYPE (temp) == type)
>>>>> + return temp;
>>>>> @@ -1430,6 +1438,8 @@ cxx_eval_call_expression (const constexpr_ctx
>>>>> *ctx,
>>>>> tree t,
>>>>> bool
>>>>> reduced_constant_expression_p (tree t)
>>>>> {
>>>>> + /* Make sure we remove useless initial NOP_EXPRs. */
>>>>> + STRIP_NOPS (t);
>>>>
>>>>
>>>> Within the constexpr code we should be folding away NOPs as they are
>>>> generated, they shouldn't live this long.
>>>
>>>
>>> Well, we might see them on overflows ...
>>
>>
>> We shouldn't within the constexpr code. NOPs for expressions that are
>> non-constant due to overflow are added in
>> cxx_eval_outermost_constant_expr, so we shouldn't see them in the middle
>> of constexpr evaluation.
>>
>>>>> @@ -1088,7 +1093,10 @@ cxx_bind_parameters_in_call (const constexpr_ctx
>>>>> *ctx, tree t,
>>>>> && is_dummy_object (x))
>>>>> {
>>>>> x = ctx->object;
>>>>> - x = cp_build_addr_expr (x, tf_warning_or_error);
>>>>> + if (x)
>>>>> + x = cp_build_addr_expr (x, tf_warning_or_error);
>>>>> + else
>>>>> + x = get_nth_callarg (t, i);
>>>>
>>>>
>>>> This still should not be necessary.
>>>
>>>
>>> Yeah, most likely. But I got initially here some issues, so I don't
>>> see that this code would worsen things.
>>
>>
>> If this code path is hit, that means something has broken my design, and
>> I don't want to just paper over that. Please revert this change.
>>
>>>>> case SIZEOF_EXPR:
>>>>> + if (processing_template_decl
>>>>> + && (!COMPLETE_TYPE_P (TREE_TYPE (t))
>>>>> + || TREE_CODE (TYPE_SIZE (TREE_TYPE (t))) != INTEGER_CST))
>>>>> + return t;
>>>>
>>>>
>>>> Why is this necessary?
>>>
>>>
>>> We don't want to resolve SIZEOF_EXPR within template-declarations for
>>> incomplete types, of if its size isn't fixed. Issue is that we
>>> otherwise get issues about expressions without existing type (as usual
>>> within template-declarations for some expressions).
>>
>>
>> Yes, but we shouldn't have gotten this far with a dependent sizeof;
>> maybe_constant_value just returns if
>> instantiation_dependent_expression_p is true.
>>
>>>>> @@ -3391,8 +3431,23 @@ cxx_eval_constant_expression (const
>>>>> constexpr_ctx
>>>>> *ctx, tree t,
>>>>> case CONVERT_EXPR:
>>>>> case VIEW_CONVERT_EXPR:
>>>>> case NOP_EXPR:
>>>>> + case UNARY_PLUS_EXPR:
>>>>> {
>>>>> + enum tree_code tcode = TREE_CODE (t);
>>>>> tree oldop = TREE_OPERAND (t, 0);
>>>>> +
>>>>> + if (tcode == NOP_EXPR && TREE_TYPE (t) == TREE_TYPE (oldop) &&
>>>>> TREE_OVERFLOW_P (oldop))
>>>>> + {
>>>>> + if (!ctx->quiet)
>>>>> + permerror (input_location, "overflow in constant
>>>>> expression");
>>>>> + /* If we're being permissive (and are in an enforcing
>>>>> + context), ignore the overflow. */
>>>>> + if (!flag_permissive)
>>>>> + *overflow_p = true;
>>>>> + *non_constant_p = true;
>>>>> +
>>>>> + return t;
>>>>> + }
>>>>> tree op = cxx_eval_constant_expression (ctx, oldop,
>>>>
>>>>
>>>> Why doesn't the call to cxx_eval_constant_expression at the bottom here
>>>> handle oldop having TREE_OVERFLOW set?
>>>
>>>
>>> I just handled the case that we see here a wrapping NOP_EXPR around an
>>> overflow. As this isn't handled by cxx_eval_constant_expression.
>>
>>
>> How does it need to be handled? A NOP_EXPR wrapped around an overflow
>> is there to indicated that the expression is non-constant, and it can't
>> be simplified any farther.
>>
>> Please give an example of what was going wrong.
>>
>>>>> @@ -565,6 +571,23 @@ cp_gimplify_expr (tree *expr_p, gimple_seq *pre_p,
>>>>> gimple_seq *post_p)
>>>>>
>>>>> switch (code)
>>>>> {
>>>>> + case SIZEOF_EXPR:
>>>>> + if (SIZEOF_EXPR_TYPE_P (*expr_p))
>>>>> + *expr_p = cxx_sizeof_or_alignof_type (TREE_TYPE (TREE_OPERAND
>>>>> (*expr_p,
>>>>> +
>>>>> 0)),
>>>>> + SIZEOF_EXPR, false);
>>>>> + else if (TYPE_P (TREE_OPERAND (*expr_p, 0)))
>>>>> + *expr_p = cxx_sizeof_or_alignof_type (TREE_OPERAND (*expr_p,
>>>>> 0),
>>>>> + SIZEOF_EXPR, false);
>>>>> + else
>>>>> + *expr_p = cxx_sizeof_or_alignof_expr (TREE_OPERAND (*expr_p,
>>>>> 0),
>>>>> + SIZEOF_EXPR, false);
>>>>> + if (*expr_p == error_mark_node)
>>>>> + *expr_p = size_one_node;
>>>>> +
>>>>> + *expr_p = maybe_constant_value (*expr_p);
>>>>> + ret = GS_OK;
>>>>> + break;
>>>>
>>>>
>>>> Why are these surviving until gimplification time?
>>>
>>>
>>> This might be still necessary. I will retest, when bootstrap works.
>>> As we now added SIZEOF_EXPR folding to cp_fold, and if we catch all
>>> expressions a sizeof can occure, this shouldn't be necessary anymore.
>>> AFAIR I saw here some issues about initialzation for global-variables,
>>> which weren't caught.
>>
>>
>> Hmm, I wonder why you would see issues with global initializers that
>> aren't seen on trunk? In any case, if the issue is with global
>> initializers, they should be handled sooner, not here.
>>
>>>>> @@ -608,9 +608,13 @@ cp_fold_convert (tree type, tree expr)
>>>>> }
>>>>> else
>>>>> {
>>>>> - conv = fold_convert (type, expr);
>>>>> + if (TREE_CODE (expr) == INTEGER_CST)
>>>>> + conv = fold_convert (type, expr);
>>>>> + else
>>>>> + conv = convert (type, expr);
>>>>
>>>>
>>>> I still think that cp_fold_convert should always call fold_convert, and
>>>> callers that we don't want to fold should call convert instead, or
>>>> another function that folds only conversion of constants. We had talked
>>>> about the name "fold_cst", but I think that name isn't very clear; would
>>>> it make sense to just have convert always fold conversions of constants?
>>>
>>>
>>> We could introduce that, but we still have the issues about some
>>> unary-operations on constants, too. So we could do for any conversion
>>> afterwards a call to cp_try_fold_to_constant, which should reflect
>>> that pretty well, beside within template-declarations ...
>
>
> Now we've been talking about calling it "fold_simple".
Yes, rename happened. I will send a patch for that. By reading
cp_fold_convert, I agree that folding can be assumed ....
>
>>>>> @@ -1529,8 +1532,11 @@ build_expr_type_conversion (int desires, tree
>>>>> expr,
>>>>> bool complain)
>>>>> tree basetype = TREE_TYPE (expr);
>>>>> tree conv = NULL_TREE;
>>>>> tree winner = NULL_TREE;
>>>>> + /* Want to see if EXPR is a constant. See below checks for
>>>>> null_node.
>>>>> */
>>>>> + tree expr_folded = cp_try_fold_to_constant (expr);
>>>>>
>>>>> - if (expr == null_node
>>>>> + STRIP_NOPS (expr_folded);
>>>>> + if (expr_folded == null_node
>>>>
>>>>
>>>> Again, we shouldn't need to fold to check for null_node, it only occurs
>>>> when explicitly written. Folding should never produce null_node unless
>>>> the argument was already null_node.
>>>
>>>
>>> Well, we need to do this for diagnostic messages AFAIR. We want to
>>> see if expression folded gets a constant, so that diagnostics getting
>>> displayed right.
>>
>>
>> Again, null_node is special. It indicates that the user typed "__null".
>> That's what we're checking for here. Folding is both unnecessary and
>> undesirable.
>>
>>>>> @@ -1548,7 +1554,7 @@ build_expr_type_conversion (int desires, tree
>>>>> expr,
>>>>> bool complain)
>>>>> switch (TREE_CODE (basetype))
>>>>> {
>>>>> case INTEGER_TYPE:
>>>>> - if ((desires & WANT_NULL) && null_ptr_cst_p (expr))
>>>>> + if ((desires & WANT_NULL) && null_ptr_cst_p (expr_folded))
>>>>
>>>>
>>>> Again, we don't want to fold before calling null_ptr_cst_p, since in
>>>> C++11 only a literal 0 is a null pointer constant. For C++98 we already
>>>> fold in null_ptr_cst_p.
>>>
>>>
>>> We need to avoid useless conversion, so we should reduce to simple
>>> constant-value ...
>>
>>
>> No. Again, in C++11 only "0" or "0L" is a null pointer constant. A
>> more complex expression that folds to 0 is NOT a null pointer constant.
>> Folding is actively harmful here.
>>
>> And again, in C++98 mode null_ptr_cst_p already folds, so doing it here
>> is redundant.
>>
>> Was I unclear?
>>
>>>>> @@ -8496,16 +8467,18 @@ compute_array_index_type (tree name, tree size,
>>>>> tsubst_flags_t complain)
>>>>> SET_TYPE_STRUCTURAL_EQUALITY (itype);
>>>>> return itype;
>>>>> }
>>>>> -
>>>>> +
>>>>> + /* We need to do fully folding to determine if we have VLA, or
>>>>> not. */
>>>>> + tree size_constant = cp_try_fold_to_constant (size);
>>>>
>>>>
>>>> Again, we already called maybe_constant_value.
>>>
>>>
>>> Sure, but maybe_constant_value still produces nops ...
>>
>>
>> If someone tries to create an array with a size that involves arithmetic
>> overflow, that's undefined behavior and we should probably give an error
>> rather than fold it away.
>>
>>>>> @@ -13078,6 +13042,8 @@ build_enumerator (tree name, tree value, tree
>>>>> enumtype, tree attributes,
>>>>> if (value)
>>>>> STRIP_TYPE_NOPS (value);
>>>>>
>>>>> + if (value)
>>>>> + value = cp_try_fold_to_constant (value);
>>>>
>>>>
>>>> Again, this is unnecessary because we call cxx_constant_value below.
>>>
>>>
>>> See nops, and other unary-operations we want to reduce here to real
>>> constant value ...
>>
>>
>> The cxx_constant_value call below will deal with them.
>
>
> Likewise for grokbitfield.
Hmm, AFAIR we don't call cxx_constant_value in all code-paths. But I
will look into it, and come back to you on it.
>>>>> @@ -13102,6 +13068,7 @@ build_enumerator (tree name, tree value, tree
>>>>> enumtype, tree attributes,
>>>>> if (value != NULL_TREE)
>>>>> {
>>>>> value = cxx_constant_value (value);
>>>>> + STRIP_NOPS (value);
>>>>
>>>>
>>>> Again, the only time a constant result should have a NOP_EXPR around it
>>>> is if it isn't really constant. Why do you want to strip that?
>>>
>>>
>>> As for an enumerator-value we might have overflows, which are silently
>>> ignored.
>>
>>
>> They shouldn't be ignored. C++ requires that the value be constant, and
>> overflow makes it non-constant.
>>
>>> I will recheck this what example we have for this when bootstrap is
>>> working again.
>>>
>>>>> @@ -6575,6 +6578,13 @@ cp_parser_postfix_open_square_expression
>>>>> (cp_parser
>>>>> *parser,
>>>>> index = cp_parser_expression (parser);
>>>>> }
>>>>>
>>>>> + /* For offsetof and declaration of types we need
>>>>> + constant integeral values.
>>>>> + Also we meed to fold for negative constants so that diagnostic in
>>>>> + c-family/c-common.c doesn't fail for array-bounds. */
>>>>> + if (for_offsetof || decltype_p
>>>>> + || (TREE_CODE (index) == NEGATE_EXPR && TREE_CODE (TREE_OPERAND
>>>>> (index, 0)) == INTEGER_CST))
>>>>> + index = cp_try_fold_to_constant (index);
>>>>
>>>>
>>>> Similarly, for offsetof the folding should happen closer to where it is
>>>> needed.
>>>>
>>>> Why is it needed for decltype, which is querying the type of an
>>>> expression?
>>>>
>>>> For NEGATE_EXPR, we had talked about always folding a NEGATE of a
>>>> constant; this isn't the right place to do it.
>>>
>>>
>>> Same as above, we need in those cases (and for -1 too) the constant
>>> values early anyway. So I saw it as more logical to have done this
>>> conversion as soon as possible after initialization.
>>
>>
>> I don't think this is as soon as possible; we can fold the NEGATE_EXPR
>> immediately when we build it, at the end of cp_build_unary_op.
>>
>> I still wonder why any folding is necessary for decltype. When I ask
>> why, I want to know *why*, not just have you tell me again that it's
>> needed. I don't think it is.
>>
>> For offsetof, I wonder if it makes sense to extend fold_offsetof_1 to
>> handle whatever additional folding is needed here. If not, then fold in
>> finish_offsetof, before calling fold_offsetof.
>
>
> I see that this is now an unconditional fold_simple, but I still don't
> understand why it needs to be folded here, in the parser.
The point to fold the 'value' here is for cases
'processing_template_decl' isn't false. We could move it to the
else-case of the 'if (! processing_template_decl)' line for being more
explicit?
>>....
>>>
>>> Anyway, if you prefer, we can do this in builder-routines, and remove
>>> at places constants aren't needed directly after parsing it those calls.
>>
>>
>> I want to delay it to:
>>
>> 1) the places where we actually care about constant values, all of which
>> already call maybe_constant_value or cxx_constant_value, so they
>> shouldn't need much change; and
>> 2) the places where we want a simplified expression for warnings, where
>> we should call fold_simple.
>
>
>> Folding in the parser is wrong, most of all because template
>> substitution doesn't go through the parser.
>
>
> There are still several folds in cp_parser_omp_* that should move later.
In 'cp_parser_omp_var_list_no_open' we need to fold 'length' can
'low_bound' as those values getting checked some lines below (see
lines 27936, 27944). We could call here fold_simple instead?
If we would delay it to later, we would need to move diagnostics here too,
In 'cp_parser_omp_clause_aligned','cp_parser_omp_clause_safelen', and
in 'cp_parser_omp_clause_simdlen' I tried to fold early to prevent
early cases assuming that OMP-operands are already folded.
I tested this, and it doesn't seems to be necessary anymore due we
perform full converatge here in the cp_fold_r walker now.
I removed here the fold-invocation of them.
In 'cp_parser_cilk_grainsize' we fold 2nd argument of
'cp_paser_cild_for' by 'fold_simple'. Not sure if it is worth to move
operand-folding into cp_parser_cilk_for itself, as we have here just
two users of 'cp_parser_cilk_for'.
One time we pass 'integer_zero_node' as this argument, and the other
time a binary-expression, which might be constant value.
But sure we can move it into 'cp_parser_cilk_grainsize'.if you prefer?
>> finish_unary_op_expr (location_t loc, enum tree_code code, tree expr,
>> tsubst_flags_t complain)
>> {
>> + tree expr_ovl = expr;
>> tree result = build_x_unary_op (loc, code, expr, complain);
>> + tree result_ovl = result;
>> +
>> + expr_ovl = fold_simple (expr_ovl);
>> + STRIP_NOPS (expr_ovl);
>
>
> Why both fold_simple and STRIP_NOPS?
If we have an overflow-value encapsulated into an nop_expr, we want to
see that overflow-expression itself.
fold_simple preserves the nop_expr conversion, so we want to remove
it, if present.
Not sure, if this really still can happen, I will do some testing on it.
>>>>> @@ -441,7 +441,7 @@ build_aggr_init_expr (tree type, tree init)
>>>>> else if (TREE_CODE (init) == AGGR_INIT_EXPR)
>>>>> fn = AGGR_INIT_EXPR_FN (init);
>>>>> else
>>>>> - return convert (type, init);
>>>>> + return fold (convert (type, init));
>>>>
>>>>
>>>> Why fold here?
>>>
>>>
>>> We had this already in prior thread. fold (convert ()) !=
>>> fold_convert () for C++. The fold is just there to make sure we fold
>>> away useless casts.
>>
>>
>> But why here? Can't we fold away useless casts earlier (in convert) or
>> later (when we care about having a simplified expression)?
>>
>>>>> @@ -3664,6 +3660,10 @@ convert_arguments (tree typelist, vec<tree,
>>>>> va_gc>
>>>>> **values, tree fndecl,
>>>>> && (type == 0 || TREE_CODE (type) != REFERENCE_TYPE))
>>>>> val = TREE_OPERAND (val, 0);
>>>>>
>>>>> + /* For BUILT_IN_NORMAL we want to fold constants. */
>>>>> + if (fndecl && DECL_BUILT_IN (fndecl)
>>>>> + && DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_NORMAL)
>>>>> + val = fold (val);
>>>>
>>>>
>>>> Why?
>>>
>>>
>>> As builtin-handlers are expecting to see constant values.
>
>
> I would think this should be maybe_constant_value then.
Why? At the end we resolve normal-builtin via 'fold_call_expr'. Of
course we can invoke here maybe_constant_value, but it would end up in
the same folding of a builtin-expression. So calling here directly
'fold' just short-cuts this.
>>>>> @@ -5026,18 +5023,21 @@ cp_build_binary_op (location_t location,
>>>>> }
>>>>>
>>>>> result = build2 (resultcode, build_type, op0, op1);
>>>>> - result = fold_if_not_in_template (result);
>>>>> if (final_type != 0)
>>>>> result = cp_convert (final_type, result, complain);
>>>>> -
>>>>> - if (TREE_OVERFLOW_P (result)
>>>>> + op0 = fold_non_dependent_expr (op0);
>>>>> + op1 = fold_non_dependent_expr (op1);
>>>>> + STRIP_NOPS (op0);
>>>>> + STRIP_NOPS (op1);
>>>>> + result_ovl = fold_build2 (resultcode, build_type, op0, op1);
>>>>> + if (TREE_OVERFLOW_P (result_ovl)
>>>>> && !TREE_OVERFLOW_P (op0)
>>>>> && !TREE_OVERFLOW_P (op1))
>>>>> - overflow_warning (location, result);
>>>>> + overflow_warning (location, result_ovl);
>>>>
>>>>
>>>> Don't you want to use cp_fully_fold here?
>>
>>
>> ?
>
>
> Introducing *_non_dependent_expr is definitely wrong here.
I don't remember anymore, why I used here *_non_dependent_expr. I
will see if we can use here instead cp_fully_fold. If we can get rid
of the STRIP_NOPs, I am not sure, as we are interested in
overflow-bit, and don't want to see it encapsulated into nop_expr ...
>
>>>>> @@ -7249,7 +7249,7 @@ gimplify_omp_for (tree *expr_p, gimple_seq
>>>>> *pre_p)
>>>>> /* Handle OMP_FOR_COND. */
>>>>> t = TREE_VEC_ELT (OMP_FOR_COND (for_stmt), i);
>>>>> gcc_assert (COMPARISON_CLASS_P (t));
>>>>> - gcc_assert (TREE_OPERAND (t, 0) == decl);
>>>>> + gcc_assert (TREE_OPERAND (t, 0) == decl || TREE_OPERAND (t,
>>>>> 1) ==
>>>>> decl);
>>>>
>>>>
>>>> Why didn't delayed folding canonicalize this so that the decl is in op0?
>>>
>>>
>>> Delay folding doesn't canonicalize this.
>>
>>
>> Why not? Doesn't it fold all expressions?
>
>
> ?
It fold them lately. I will recheck this code-change. It might be no
longer required due recent changes to omp-folding. It could be that
original pattern didn't applied here anymore, and therefore statement
didn't been transformed into its canonical form. Bit I assume this
could be resolved.
>>> Actually we don't want to touch here anything in parsered tree. We
>>> could do this in generalization-pass before gimplification. Seems to
>>> be something we don't catch for now, which makes me wonder a bit.
>>>
>>>>> @@ -508,7 +508,9 @@ extract_omp_for_data (gomp_for *for_stmt, struct
>>>>> omp_for_data *fd,
>>>>> gcc_assert (gimple_omp_for_kind (for_stmt)
>>>>> == GF_OMP_FOR_KIND_CILKSIMD
>>>>> || (gimple_omp_for_kind (for_stmt)
>>>>> - == GF_OMP_FOR_KIND_CILKFOR));
>>>>> + == GF_OMP_FOR_KIND_CILKFOR)
>>>>> + || (gimple_omp_for_kind (for_stmt)
>>>>> + == GF_OMP_FOR_KIND_FOR));
>>>>
>>>>
>>>> This still seems like a red flag; how is delayed folding changing the
>>>> OMP for kind?
>>>
>>>
>>> It doesn't. The issue is that some canonical operations of fold
>>> aren't happening anymore on which omp depends.
>>
>>
>> That seems like a problem.
See above. I will come up on this tomorrow.
>> @@ -867,7 +867,7 @@ expand_subword_shift (machine_mode op1_mode, optab
>> binoptab,
>> are truncated to the mode size. */
>> carries = expand_binop (word_mode, reverse_unsigned_shift,
>> outof_input, const1_rtx, 0, unsignedp,
>> methods);
>> - if (shift_mask == BITS_PER_WORD - 1)
>> + if (shift_mask == (unsigned HOST_WIDE_INT) (BITS_PER_WORD - 1))
>
>
> These should still be unnecessary.
Yes, they are. We handle now the avoiding of dead-code for constants
(cond-expression, truthif* expressions, and useless convert-warnings).
So this change is something only interesting for different compiler,
but not related to delayed-folding anymore.
I will check, and remove it tomorrow.
>>>>> @@ -1947,6 +1947,8 @@ build_complex (tree type, tree real, tree imag)
>>>>> {
>>>>> tree t = make_node (COMPLEX_CST);
>>>>>
>>>>> + real = fold (real);
>>>>> + imag = fold (imag);
>>>>
>>>>
>>>> I still think this is wrong. The arguments should be sufficiently
>>>> folded.
>>>
>>>
>>> As we don't fold unary-operators on constants, we need to fold it at
>>> some place. AFAICS is the C++ FE not calling directly build_complex.
>>> So this place was the easiest way to avoid issues with things like '-'
>>> '1' etc.
>>
>>
>> Is this because of the
>>>
>>> value = build_complex (NULL_TREE, convert (const_type,
>>> integer_zero_node),
>>> value);
Might be. This should be indeed a 'fold_convert', isn't it?
>> in interpret_float? I think "convert" definitely needs to do some
>> folding, since it's called from middle-end code that expects that.
>
>
> I remember talking about "convert" doing some folding (and cp_convert not)
> in our 1:1 last week.
>
Can't remember that. I know that we were talking about the difference
of convert and fold_convert. convert can be used on C++ specifics,
but fold_convert is something shared with ME. So first 'fold_convert'
isn't the same as 'fold (convert ())'.
I don't find places we invoke convert () in ME. We have some calls in
convert.c (see convert_to_integer, convert_to_integer_nofold, and
convert_to_real), which all used in AST only AFAICS.
I remember that we were talking about adding a standard-folding to
convert for operations on constant-values (as we do for
convert_to_integer). Do you mean this?
>>>>> @@ -5080,6 +5081,7 @@ output_constructor_bitfield (oc_local_state
>>>>> *local,
>>>>> unsigned int bit_offset)
>>>>> while (TREE_CODE (local->val) == VIEW_CONVERT_EXPR
>>>>> || TREE_CODE (local->val) == NON_LVALUE_EXPR)
>>>>> local->val = TREE_OPERAND (local->val, 0);
>>>>> + local->val = fold (local->val);
>>>>
>>>>
>>>> Likewise.
>>>
>>>
>>> As soon as we can be sure that values getting fully_folded, or at
>>> least folded for constants, we should be able to remove this.
>>
>>
>> Yep, they need to be folded before we get here.
>>
>> It looks like your latest checkin added more redundant folding:
>>
>>> @@ -3311,6 +3311,9 @@ finish_case_label (location_t loc, tree
>>> low_value, tree hi
>>> gh_value)
>>> low_value = case_conversion (type, low_value);
>>> high_value = case_conversion (type, high_value);
>>>
>>> + low_value = cp_fully_fold (low_value);
>>> + high_value = cp_fully_fold (high_value);
>>
>>
>> Again, case_conversion should have already folded constants.
>>
>>> @@ -5776,6 +5776,8 @@ convert_nontype_argument (tree type, tree expr,
>>> tsubst_flags_t complain)
>>> {
>>> tree expr_type;
>>>
>>> + expr = cp_try_fold_to_constant (expr);
>>> +
>>> /* Detect immediately string literals as invalid non-type argument.
>>> This special-case is not needed for correctness (we would easily
>>> catch this later), but only to provide better diagnostic for this
>>> @@ -5852,6 +5854,7 @@ convert_nontype_argument (tree type, tree expr,
>>> tsubst_flags_t complain)
>>> else if (TYPE_PTR_OR_PTRMEM_P (type))
>>> {
>>> tree folded = maybe_constant_value (expr);
>>> + folded = cp_try_fold_to_constant (expr);
>>
>>
>> And here, convert_nontype_argument already uses
>> maybe_constant_value/cxx_constant_value for folding constants.
>
>
> Jason
>
Thanks,
Kai
