Re: C++ PATCH for c++/56930 (wrong -Wconversion warning with sizeof)

[Jakub Jelinek <jakub at redhat dot com>]

On Wed, May 22, 2013 at 11:08:40PM -0400, Jason Merrill wrote:
> Another issue with delayed folding of sizeof.  On the trunk, we
> should fold everything before passing it on to
> warnings_for_convert_and_check; on the branch, I'm inclined to be
> conservative and only fold SIZEOF_EXPR.
> 
> Jakub, do you have an opinion about whether this should go into
> 4.8.1 or 4.8.2?

I'd say it can wait for 4.8.2 if you don't mind.

> commit 0477e34118f5bdbdb4d93c795f59d122dca02f36
> Author: Jason Merrill <jason@redhat.com>
> Date:   Wed May 22 10:10:49 2013 -0400
> 
>     	PR c++/56930
>     	* call.c (convert_like_real): Use cp_convert_and_check.
>     	* cvt.c (cp_convert_and_check): Use maybe_constant_value.
>     	* semantics.c (cxx_eval_constant_expression): Handle LTGT_EXPR.
>     	(potential_constant_expression_1): Handle OMP_ATOMIC*.
> 
> diff --git a/gcc/cp/call.c b/gcc/cp/call.c
> index 71a1589..0b6a83f 100644
> --- a/gcc/cp/call.c
> +++ b/gcc/cp/call.c
> @@ -6199,10 +6199,10 @@ convert_like_real (conversion *convs, tree expr, tree fn, int argnum,
>    if (convs->check_narrowing)
>      check_narrowing (totype, expr);
>  
> -  if (issue_conversion_warnings && (complain & tf_warning))
> -    expr = convert_and_check (totype, expr);
> +  if (issue_conversion_warnings)
> +    expr = cp_convert_and_check (totype, expr, complain);
>    else
> -    expr = convert (totype, expr);
> +    expr = cp_convert (totype, expr, complain);
>  
>    return expr;
>  }
> diff --git a/gcc/cp/cvt.c b/gcc/cp/cvt.c
> index 93be76a..d9e905e 100644
> --- a/gcc/cp/cvt.c
> +++ b/gcc/cp/cvt.c
> @@ -624,10 +624,20 @@ cp_convert_and_check (tree type, tree expr, tsubst_flags_t complain)
>    result = cp_convert (type, expr, complain);
>  
>    if ((complain & tf_warning)
> -      && c_inhibit_evaluation_warnings == 0
> -      && !TREE_OVERFLOW_P (expr)
> -      && result != error_mark_node)
> -    warnings_for_convert_and_check (type, expr, result);
> +      && c_inhibit_evaluation_warnings == 0)
> +    {

I'm surprised there is no fold_nondependent_expr_sfinae (expr, tf_none);
first, we had to add that before to similar maybe_constant_value calls
for -Wdiv-by-zero, shift warnings etc.  Perhaps we can have a helper
that will call the above, maybe_constant_value and strip the possible
NOP_EXPR that maybe_constant_value can add, and use that in all the places
where we want for diagnostic purposes try to fold everything as much as
possible.

> +      tree folded = maybe_constant_value (expr);
> +      tree stripped = folded;
> +      tree folded_result = cp_convert (type, folded, complain);
> +
> +      /* maybe_constant_value wraps an INTEGER_CST with TREE_OVERFLOW in a
> +	 NOP_EXPR so that it isn't TREE_CONSTANT anymore.  */
> +      STRIP_NOPS (stripped);
> +
> +      if (!TREE_OVERFLOW_P (stripped)
> +	  && folded_result != error_mark_node)
> +	warnings_for_convert_and_check (type, folded, folded_result);
> +    }

> --- a/gcc/cp/cvt.c
> +++ b/gcc/cp/cvt.c
> @@ -620,6 +620,9 @@ cp_convert_and_check (tree type, tree expr, tsubst_flags_t complain)
>  
>    if (TREE_TYPE (expr) == type)
>      return expr;
> +
> +  if (TREE_CODE (expr) == SIZEOF_EXPR)
> +    expr = maybe_constant_value (expr);

Is this sufficient though?  What if the testcase is:
int main()
{
  int x = sizeof(int) + 1;
  int y { sizeof(int) * __CHAR_BIT__ };
  int z = sizeof(int) + 2 * sizeof(y) + 3 * sizeof (x);
}

	Jakub