Re: C++ PATCH to detect narrowing in case values (PR c++/90805)

[Marek Polacek <polacek at redhat dot com>]

Ping.

On Wed, Jul 17, 2019 at 04:05:24PM -0400, Marek Polacek wrote:
> On Wed, Jul 03, 2019 at 01:57:06PM -0400, Jason Merrill wrote:
> > On 7/3/19 10:13 AM, Marek Polacek wrote:
> > > On Sat, Jun 22, 2019 at 11:28:36PM -0400, Jason Merrill wrote:
> > > > On 6/13/19 5:03 PM, Marek Polacek wrote:
> > > > > Case values are converted constant expressions, so narrowing conversion is not
> > > > > permitted.  This patch adds detecting narrowing to case_conversion; it's a
> > > > > handy spot because we have both the value and the (adjusted) type of the
> > > > > condition.
> > > > 
> > > > Is there a reason not to use build_converted_constant_expr?
> > > 
> > > The function comment says "Note that if TYPE and VALUE are already integral
> > > we don't really do the conversion because the language-independent
> > > warning/optimization code will work better that way" so I avoided adding any
> > > conversions.
> > 
> > > What I could do is to, instead of calling check_narrowing, call
> > > build_converted_constant_expr (type, value, tf_warning_or_error);
> > > and not use its result, but I'm not sure what the benefits would be.
> > 
> > I was thinking about using it instead of the current
> > perform_implicit_conversion_flags, so we get the somewhat different
> > constraints on the conversion.  And then it becomes simpler to use it
> > unconditionally but throw the result away in the easy case.
> 
> Ah, I see.  So something like this?
> 
> Bootstrapped/regtested on x86_64-linux, ok for trunk?
> 
> 2019-07-17  Marek Polacek  <polacek@redhat.com>
> 
> 	PR c++/90805 - detect narrowing in case values.
> 	* decl.c (case_conversion): Detect narrowing in case values.
> 
> 	* c-c++-common/pr89888.c: Update expected dg-error.
> 	* g++.dg/cpp0x/Wnarrowing17.C: New test.
> 	* g++.dg/cpp0x/enum28.C: Update expected dg-error.
> 
> diff --git gcc/cp/decl.c gcc/cp/decl.c
> index dbcf681c783..2d3ffdfbb54 100644
> --- gcc/cp/decl.c
> +++ gcc/cp/decl.c
> @@ -3630,16 +3630,23 @@ case_conversion (tree type, tree value)
>  
>    value = mark_rvalue_use (value);
>  
> +  if (INTEGRAL_OR_UNSCOPED_ENUMERATION_TYPE_P (type))
> +    type = type_promotes_to (type);
> +
> +  tree ovalue = value;
> +  /* The constant-expression VALUE shall be a converted constant expression
> +     of the adjusted type of the switch condition, which doesn't allow
> +     narrowing conversions.  */
> +  value = build_converted_constant_expr (type, value, tf_warning_or_error);
> +
>    if (cxx_dialect >= cxx11
>        && (SCOPED_ENUM_P (type)
> -	  || !INTEGRAL_OR_UNSCOPED_ENUMERATION_TYPE_P (TREE_TYPE (value))))
> -    {
> -      if (INTEGRAL_OR_UNSCOPED_ENUMERATION_TYPE_P (type))
> -	type = type_promotes_to (type);
> -      value = (perform_implicit_conversion_flags
> -	       (type, value, tf_warning_or_error,
> -		LOOKUP_IMPLICIT | LOOKUP_NO_NON_INTEGRAL));
> -    }
> +	  || !INTEGRAL_OR_UNSCOPED_ENUMERATION_TYPE_P (TREE_TYPE (ovalue))))
> +    /* Use the converted value.  */;
> +  else
> +    /* The already integral case.  */
> +    value = ovalue;
> +
>    return cxx_constant_value (value);
>  }
>  
> diff --git gcc/testsuite/c-c++-common/pr89888.c gcc/testsuite/c-c++-common/pr89888.c
> index d9e11d6f26a..f14881ca052 100644
> --- gcc/testsuite/c-c++-common/pr89888.c
> +++ gcc/testsuite/c-c++-common/pr89888.c
> @@ -11,8 +11,8 @@ foo (unsigned char x)
>      {
>      case -1: y = -1; break;			/* { dg-message "previously used here" } */
>  						/* { dg-warning "case label value is less than minimum value for type" "" { target *-*-* } .-1 } */
> -    case 0xffffffff: y = 0xffffffff; break;	/* { dg-error "duplicate case value" } */
> -    case ~0U: y = ~0U; break;			/* { dg-error "duplicate case value" } */
> +    case 0xffffffff: y = 0xffffffff; break;	/* { dg-error "duplicate case value|narrowing" } */
> +    case ~0U: y = ~0U; break;			/* { dg-error "duplicate case value|narrowing" } */
>      }
>  }
>  
> diff --git gcc/testsuite/g++.dg/cpp0x/Wnarrowing17.C gcc/testsuite/g++.dg/cpp0x/Wnarrowing17.C
> new file mode 100644
> index 00000000000..064de531cb3
> --- /dev/null
> +++ gcc/testsuite/g++.dg/cpp0x/Wnarrowing17.C
> @@ -0,0 +1,19 @@
> +// PR c++/90805 - detect narrowing in case values.
> +// { dg-do compile { target c++11 } }
> +
> +void f(int i, char c, unsigned u)
> +{
> +  switch (i)
> +    {
> +    case 2149056512u:; // { dg-error "narrowing conversion of .2149056512. from .unsigned int. to .int." }
> +    case (long long int) 1e10:; // { dg-error "narrowing conversion of .10000000000. from .long long int. to .int." }
> +    // { dg-warning "overflow in conversion" "overflow" { target *-*-* } .-1 }
> +    }
> +
> +  switch (c)
> +    // No narrowing, the adjusted type is int.
> +    case 300:; // { dg-warning "exceeds maximum value for type" }
> +
> +  switch (u)
> +    case -42:; // { dg-error "narrowing conversion of .-42. from .int. to .unsigned int." }
> +}
> diff --git gcc/testsuite/g++.dg/cpp0x/enum28.C gcc/testsuite/g++.dg/cpp0x/enum28.C
> index 3967699dd03..bfebde57cb3 100644
> --- gcc/testsuite/g++.dg/cpp0x/enum28.C
> +++ gcc/testsuite/g++.dg/cpp0x/enum28.C
> @@ -7,11 +7,11 @@ void f(int i)
>  {
>    switch (i)
>      {
> -    case 1.0:;    // { dg-error "could not convert" }
> +    case 1.0:;    // { dg-error "could not convert|conversion from" }
>      }
>  
>    switch (i)
>      {
> -    case g():;    // { dg-error "could not convert" }
> +    case g():;    // { dg-error "could not convert|conversion from" }
>      }
>  }

Marek