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

login
register
mail settings
Submitter Jason Merrill
Date May 23, 2013, 3:08 a.m.
Message ID <519D8838.7020806@redhat.com>
Download mbox | patch
Permalink /patch/245799/
State New
Headers show

Comments

Jason Merrill - May 23, 2013, 3:08 a.m.
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?
Jakub Jelinek - May 23, 2013, 5:26 a.m.
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
Jason Merrill - May 23, 2013, 12:42 p.m.
On 05/23/2013 01:26 AM, Jakub Jelinek wrote:
> Is this sufficient though?

No, but it handles the most common case and is safer than the version on 
the trunk, which already required me to fix a couple of holes in the 
constexpr code.  If no more holes turn up, we could move the trunk 
version to the branch later.

Jason

Patch

commit 180efaf40be900c8c95ae9d0ec237b98b17d1028
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 72c1dac..9275133 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -6195,8 +6195,8 @@  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);
 
diff --git a/gcc/cp/cvt.c b/gcc/cp/cvt.c
index 348e608..f83f370 100644
--- 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);
   
   result = cp_convert (type, expr, complain);
 
diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
index 8a7fe38..0420a41 100644
--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -7990,6 +7990,7 @@  cxx_eval_constant_expression (const constexpr_call *call, tree t,
     case UNGT_EXPR:
     case UNGE_EXPR:
     case UNEQ_EXPR:
+    case LTGT_EXPR:
     case RANGE_EXPR:
     case COMPLEX_EXPR:
       r = cxx_eval_binary_expression (call, t, allow_non_constant, addr,
@@ -8846,6 +8847,12 @@  potential_constant_expression_1 (tree t, bool want_rval, tsubst_flags_t flags)
 	}
       return false;
 
+    case OMP_ATOMIC:
+    case OMP_ATOMIC_READ:
+    case OMP_ATOMIC_CAPTURE_OLD:
+    case OMP_ATOMIC_CAPTURE_NEW:
+      return false;
+
     default:
       if (objc_is_property_ref (t))
 	return false;
diff --git a/gcc/testsuite/g++.dg/cpp0x/initlist71.C b/gcc/testsuite/g++.dg/cpp0x/initlist71.C
new file mode 100644
index 0000000..1d14390
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/initlist71.C
@@ -0,0 +1,10 @@ 
+// PR c++/56930
+// { dg-require-effective-target c++11 }
+// { dg-options -Wconversion }
+
+int main()
+{
+  int x = sizeof(int);
+  int y { sizeof(int) };
+}
+