[google/gcc-4_8] Port -Wreal-conversion warning

On Mon, Jun 24, 2013 at 9:14 PM, Xinliang David Li <davidxl@google.com> wrote:
> To avoid printing twice, can you just do
>
> opt_type = (warn_conversion ? OPT_Wconversion : OPT_Wreal_conversion);
> warning_at (loc, opt_type, ...);

Thanks for the suggestion. I have updated the enclosed patch and
retested. Okay for google/gcc-4_8?

Thanks,
Sharad

2013-06-24    <lcwu@google.com>

        * doc/invoke.texi: Document new option -Wreal-conversion.
        * c-family/c.opt: Handle new option.
        * c-family/c-opts.c (c_common_post_options): Ditto.
        * c-family/c-common.c (conversion_warning): Ditto.

testsuite/ChangeLog:

        * testsuite/gcc.dg/Wreal-conversion-1.c: New test.
        * testsuite/g++.dg/warn/Wreal-conversion-1.C: Ditto.

ok.

David

On Mon, Jun 24, 2013 at 10:03 PM, Sharad Singhai <singhai@google.com> wrote:
> On Mon, Jun 24, 2013 at 9:14 PM, Xinliang David Li <davidxl@google.com> wrote:
>> To avoid printing twice, can you just do
>>
>> opt_type = (warn_conversion ? OPT_Wconversion : OPT_Wreal_conversion);
>> warning_at (loc, opt_type, ...);
>
> Thanks for the suggestion. I have updated the enclosed patch and
> retested. Okay for google/gcc-4_8?
>
> Thanks,
> Sharad
>
> 2013-06-24    <lcwu@google.com>
>
>         * doc/invoke.texi: Document new option -Wreal-conversion.
>         * c-family/c.opt: Handle new option.
>         * c-family/c-opts.c (c_common_post_options): Ditto.
>         * c-family/c-common.c (conversion_warning): Ditto.
>
> testsuite/ChangeLog:
>
>         * testsuite/gcc.dg/Wreal-conversion-1.c: New test.
>         * testsuite/g++.dg/warn/Wreal-conversion-1.C: Ditto.
>
> Index: doc/invoke.texi
> ===================================================================
> --- doc/invoke.texi (revision 200359)
> +++ doc/invoke.texi (working copy)
> @@ -237,7 +237,7 @@ Objective-C and Objective-C++ Dialects}.
>  -Wno-attributes -Wno-builtin-macro-redefined @gol
>  -Wc++-compat -Wc++11-compat -Wcast-align  -Wcast-qual  @gol
>  -Wchar-subscripts -Wclobbered  -Wcomment @gol
> --Wconversion  -Wcoverage-mismatch  -Wno-cpp  -Wno-deprecated  @gol
> +-Wconversion -Wreal-conversion -Wcoverage-mismatch  -Wno-cpp
> -Wno-deprecated  @gol
>  -Wno-deprecated-declarations -Wdisabled-optimization  @gol
>  -Wno-div-by-zero -Wdouble-promotion -Wempty-body  -Wenum-compare @gol
>  -Wno-endif-labels -Werror  -Werror=* @gol
> @@ -4452,6 +4452,12 @@ reference to them. Warnings about conversions betw
>  unsigned integers are disabled by default in C++ unless
>  @option{-Wsign-conversion} is explicitly enabled.
>
> +@item -Wreal-conversion
> +@opindex Wreal-conversion
> +@opindex Wno-real-conversion
> +Warn for implicit type conversions from real (@code{double} or @code{float})
> +to integral values.
> +
>  @item -Wno-conversion-null @r{(C++ and Objective-C++ only)}
>  @opindex Wconversion-null
>  @opindex Wno-conversion-null
> Index: testsuite/gcc.dg/Wreal-conversion-1.c
> ===================================================================
> --- testsuite/gcc.dg/Wreal-conversion-1.c (revision 0)
> +++ testsuite/gcc.dg/Wreal-conversion-1.c (revision 0)
> @@ -0,0 +1,23 @@
> +// { dg-do compile }
> +// { dg-options "-Wreal-conversion" }
> +
> +#include <stddef.h>
> +
> +int func1(int a) {
> +  double f = a;
> +  return f;           // { dg-warning "conversion to" }
> +}
> +
> +double func3();
> +
> +void func2() {
> +  double g = 3.2;
> +  float f;
> +  int t = g;          // { dg-warning "conversion to" }
> +  int p;
> +  p = f;              // { dg-warning "conversion to" }
> +  func1(g);           // { dg-warning "conversion to" }
> +  char c = f;         // { dg-warning "conversion to" }
> +  int q;
> +  q = func3();        // { dg-warning "conversion to" }
> +}
> Index: testsuite/g++.dg/warn/Wreal-conversion-1.C
> ===================================================================
> --- testsuite/g++.dg/warn/Wreal-conversion-1.C (revision 0)
> +++ testsuite/g++.dg/warn/Wreal-conversion-1.C (revision 0)
> @@ -0,0 +1,24 @@
> +// { dg-do compile }
> +// { dg-options "-Wreal-conversion" }
> +
> +#include <stddef.h>
> +
> +int func1(int a) {
> +  double f = a;
> +  return f;           // { dg-warning "conversion to" }
> +}
> +
> +double func3();
> +
> +void func2() {
> +  double g = 3.2;
> +  float f;
> +  int t = g;          // { dg-warning "conversion to" }
> +  bool b = g;
> +  int p;
> +  p = f;              // { dg-warning "conversion to" }
> +  func1(g);           // { dg-warning "conversion to" }
> +  char c = f;         // { dg-warning "conversion to" }
> +  int q;
> +  q = func3();        // { dg-warning "conversion to" }
> +}
> Index: c-family/c.opt
> ===================================================================
> --- c-family/c.opt (revision 200359)
> +++ c-family/c.opt (working copy)
> @@ -677,6 +677,10 @@ Wsign-compare
>  C ObjC C++ ObjC++ EnabledBy(Wextra)
>  ;
>
> +Wreal-conversion
> +C ObjC C++ ObjC++ Var(warn_real_conversion) Init(-1) Warning
> +Warn for implicit type conversions from real to integral values
> +
>  Wsign-conversion
>  C ObjC C++ ObjC++ Var(warn_sign_conversion) LangEnabledBy(C ObjC,Wconversion)
>  Warn for implicit type conversions between signed and unsigned integers
> Index: c-family/c-opts.c
> ===================================================================
> --- c-family/c-opts.c (revision 200359)
> +++ c-family/c-opts.c (working copy)
> @@ -876,6 +876,12 @@ c_common_post_options (const char **pfilename)
>    if (warn_packed_bitfield_compat == -1)
>      warn_packed_bitfield_compat = 1;
>
> +  /* Enable warning for converting real values to integral values
> +     when -Wconversion is specified (unless disabled through
> +     -Wno-real-conversion).  */
> +  if (warn_real_conversion == -1)
> +    warn_real_conversion = warn_conversion;
> +
>    /* Special format checking options don't work without -Wformat; warn if
>       they are used.  */
>    if (!warn_format)
> Index: c-family/c-common.c
> ===================================================================
> --- c-family/c-common.c (revision 200359)
> +++ c-family/c-common.c (working copy)
> @@ -2668,7 +2668,7 @@ conversion_warning (tree type, tree expr)
>    tree expr_type = TREE_TYPE (expr);
>    location_t loc = EXPR_LOC_OR_HERE (expr);
>
> -  if (!warn_conversion && !warn_sign_conversion)
> +  if (!warn_conversion && !warn_sign_conversion && !warn_real_conversion)
>      return;
>
>    switch (TREE_CODE (expr))
> @@ -2715,9 +2715,12 @@ conversion_warning (tree type, tree expr)
>
>      default: /* 'expr' is not a constant.  */
>        if (unsafe_conversion_p (type, expr, true))
> - warning_at (loc, OPT_Wconversion,
> -    "conversion to %qT from %qT may alter its value",
> -    type, expr_type);
> +      {
> +        int warn_type = (warn_conversion ? OPT_Wconversion :
> OPT_Wreal_conversion);
> +        warning_at (loc, warn_type,
> +                    "conversion to %qT from %qT may alter its value",
> +                    type, expr_type);
> +      }
>      }
>  }

The warn_type should also be guarded with float type check:

 warn_type = (warn_real_conversion
&& (FLOAT_TYPE_P (type) || FLOAT_TYPE_P (expr_type)))
? OPT_Wreal_conversion
: OPT_Wconversion;

Also why did you put the warn_type code inside the default?

David


On Mon, Jun 24, 2013 at 10:03 PM, Sharad Singhai <singhai@google.com> wrote:
> On Mon, Jun 24, 2013 at 9:14 PM, Xinliang David Li <davidxl@google.com> wrote:
>> To avoid printing twice, can you just do
>>
>> opt_type = (warn_conversion ? OPT_Wconversion : OPT_Wreal_conversion);
>> warning_at (loc, opt_type, ...);
>
> Thanks for the suggestion. I have updated the enclosed patch and
> retested. Okay for google/gcc-4_8?
>
> Thanks,
> Sharad
>
> 2013-06-24    <lcwu@google.com>
>
>         * doc/invoke.texi: Document new option -Wreal-conversion.
>         * c-family/c.opt: Handle new option.
>         * c-family/c-opts.c (c_common_post_options): Ditto.
>         * c-family/c-common.c (conversion_warning): Ditto.
>
> testsuite/ChangeLog:
>
>         * testsuite/gcc.dg/Wreal-conversion-1.c: New test.
>         * testsuite/g++.dg/warn/Wreal-conversion-1.C: Ditto.
>
> Index: doc/invoke.texi
> ===================================================================
> --- doc/invoke.texi (revision 200359)
> +++ doc/invoke.texi (working copy)
> @@ -237,7 +237,7 @@ Objective-C and Objective-C++ Dialects}.
>  -Wno-attributes -Wno-builtin-macro-redefined @gol
>  -Wc++-compat -Wc++11-compat -Wcast-align  -Wcast-qual  @gol
>  -Wchar-subscripts -Wclobbered  -Wcomment @gol
> --Wconversion  -Wcoverage-mismatch  -Wno-cpp  -Wno-deprecated  @gol
> +-Wconversion -Wreal-conversion -Wcoverage-mismatch  -Wno-cpp
> -Wno-deprecated  @gol
>  -Wno-deprecated-declarations -Wdisabled-optimization  @gol
>  -Wno-div-by-zero -Wdouble-promotion -Wempty-body  -Wenum-compare @gol
>  -Wno-endif-labels -Werror  -Werror=* @gol
> @@ -4452,6 +4452,12 @@ reference to them. Warnings about conversions betw
>  unsigned integers are disabled by default in C++ unless
>  @option{-Wsign-conversion} is explicitly enabled.
>
> +@item -Wreal-conversion
> +@opindex Wreal-conversion
> +@opindex Wno-real-conversion
> +Warn for implicit type conversions from real (@code{double} or @code{float})
> +to integral values.
> +
>  @item -Wno-conversion-null @r{(C++ and Objective-C++ only)}
>  @opindex Wconversion-null
>  @opindex Wno-conversion-null
> Index: testsuite/gcc.dg/Wreal-conversion-1.c
> ===================================================================
> --- testsuite/gcc.dg/Wreal-conversion-1.c (revision 0)
> +++ testsuite/gcc.dg/Wreal-conversion-1.c (revision 0)
> @@ -0,0 +1,23 @@
> +// { dg-do compile }
> +// { dg-options "-Wreal-conversion" }
> +
> +#include <stddef.h>
> +
> +int func1(int a) {
> +  double f = a;
> +  return f;           // { dg-warning "conversion to" }
> +}
> +
> +double func3();
> +
> +void func2() {
> +  double g = 3.2;
> +  float f;
> +  int t = g;          // { dg-warning "conversion to" }
> +  int p;
> +  p = f;              // { dg-warning "conversion to" }
> +  func1(g);           // { dg-warning "conversion to" }
> +  char c = f;         // { dg-warning "conversion to" }
> +  int q;
> +  q = func3();        // { dg-warning "conversion to" }
> +}
> Index: testsuite/g++.dg/warn/Wreal-conversion-1.C
> ===================================================================
> --- testsuite/g++.dg/warn/Wreal-conversion-1.C (revision 0)
> +++ testsuite/g++.dg/warn/Wreal-conversion-1.C (revision 0)
> @@ -0,0 +1,24 @@
> +// { dg-do compile }
> +// { dg-options "-Wreal-conversion" }
> +
> +#include <stddef.h>
> +
> +int func1(int a) {
> +  double f = a;
> +  return f;           // { dg-warning "conversion to" }
> +}
> +
> +double func3();
> +
> +void func2() {
> +  double g = 3.2;
> +  float f;
> +  int t = g;          // { dg-warning "conversion to" }
> +  bool b = g;
> +  int p;
> +  p = f;              // { dg-warning "conversion to" }
> +  func1(g);           // { dg-warning "conversion to" }
> +  char c = f;         // { dg-warning "conversion to" }
> +  int q;
> +  q = func3();        // { dg-warning "conversion to" }
> +}
> Index: c-family/c.opt
> ===================================================================
> --- c-family/c.opt (revision 200359)
> +++ c-family/c.opt (working copy)
> @@ -677,6 +677,10 @@ Wsign-compare
>  C ObjC C++ ObjC++ EnabledBy(Wextra)
>  ;
>
> +Wreal-conversion
> +C ObjC C++ ObjC++ Var(warn_real_conversion) Init(-1) Warning
> +Warn for implicit type conversions from real to integral values
> +
>  Wsign-conversion
>  C ObjC C++ ObjC++ Var(warn_sign_conversion) LangEnabledBy(C ObjC,Wconversion)
>  Warn for implicit type conversions between signed and unsigned integers
> Index: c-family/c-opts.c
> ===================================================================
> --- c-family/c-opts.c (revision 200359)
> +++ c-family/c-opts.c (working copy)
> @@ -876,6 +876,12 @@ c_common_post_options (const char **pfilename)
>    if (warn_packed_bitfield_compat == -1)
>      warn_packed_bitfield_compat = 1;
>
> +  /* Enable warning for converting real values to integral values
> +     when -Wconversion is specified (unless disabled through
> +     -Wno-real-conversion).  */
> +  if (warn_real_conversion == -1)
> +    warn_real_conversion = warn_conversion;
> +
>    /* Special format checking options don't work without -Wformat; warn if
>       they are used.  */
>    if (!warn_format)
> Index: c-family/c-common.c
> ===================================================================
> --- c-family/c-common.c (revision 200359)
> +++ c-family/c-common.c (working copy)
> @@ -2668,7 +2668,7 @@ conversion_warning (tree type, tree expr)
>    tree expr_type = TREE_TYPE (expr);
>    location_t loc = EXPR_LOC_OR_HERE (expr);
>
> -  if (!warn_conversion && !warn_sign_conversion)
> +  if (!warn_conversion && !warn_sign_conversion && !warn_real_conversion)
>      return;
>
>    switch (TREE_CODE (expr))
> @@ -2715,9 +2715,12 @@ conversion_warning (tree type, tree expr)
>
>      default: /* 'expr' is not a constant.  */
>        if (unsafe_conversion_p (type, expr, true))
> - warning_at (loc, OPT_Wconversion,
> -    "conversion to %qT from %qT may alter its value",
> -    type, expr_type);
> +      {
> +        int warn_type = (warn_conversion ? OPT_Wconversion :
> OPT_Wreal_conversion);
> +        warning_at (loc, warn_type,
> +                    "conversion to %qT from %qT may alter its value",
> +                    type, expr_type);
> +      }
>      }
>  }

Sorry, my port was bad. I am going to revert this patch and redo it.

Thanks,
Sharad

On Wed, Jun 26, 2013 at 4:03 PM, Xinliang David Li <davidxl@google.com> wrote:
> The warn_type should also be guarded with float type check:
>
>  warn_type = (warn_real_conversion
> && (FLOAT_TYPE_P (type) || FLOAT_TYPE_P (expr_type)))
> ? OPT_Wreal_conversion
> : OPT_Wconversion;
>
> Also why did you put the warn_type code inside the default?
>
> David
>
>
> On Mon, Jun 24, 2013 at 10:03 PM, Sharad Singhai <singhai@google.com> wrote:
>> On Mon, Jun 24, 2013 at 9:14 PM, Xinliang David Li <davidxl@google.com> wrote:
>>> To avoid printing twice, can you just do
>>>
>>> opt_type = (warn_conversion ? OPT_Wconversion : OPT_Wreal_conversion);
>>> warning_at (loc, opt_type, ...);
>>
>> Thanks for the suggestion. I have updated the enclosed patch and
>> retested. Okay for google/gcc-4_8?
>>
>> Thanks,
>> Sharad
>>
>> 2013-06-24    <lcwu@google.com>
>>
>>         * doc/invoke.texi: Document new option -Wreal-conversion.
>>         * c-family/c.opt: Handle new option.
>>         * c-family/c-opts.c (c_common_post_options): Ditto.
>>         * c-family/c-common.c (conversion_warning): Ditto.
>>
>> testsuite/ChangeLog:
>>
>>         * testsuite/gcc.dg/Wreal-conversion-1.c: New test.
>>         * testsuite/g++.dg/warn/Wreal-conversion-1.C: Ditto.
>>
>> Index: doc/invoke.texi
>> ===================================================================
>> --- doc/invoke.texi (revision 200359)
>> +++ doc/invoke.texi (working copy)
>> @@ -237,7 +237,7 @@ Objective-C and Objective-C++ Dialects}.
>>  -Wno-attributes -Wno-builtin-macro-redefined @gol
>>  -Wc++-compat -Wc++11-compat -Wcast-align  -Wcast-qual  @gol
>>  -Wchar-subscripts -Wclobbered  -Wcomment @gol
>> --Wconversion  -Wcoverage-mismatch  -Wno-cpp  -Wno-deprecated  @gol
>> +-Wconversion -Wreal-conversion -Wcoverage-mismatch  -Wno-cpp
>> -Wno-deprecated  @gol
>>  -Wno-deprecated-declarations -Wdisabled-optimization  @gol
>>  -Wno-div-by-zero -Wdouble-promotion -Wempty-body  -Wenum-compare @gol
>>  -Wno-endif-labels -Werror  -Werror=* @gol
>> @@ -4452,6 +4452,12 @@ reference to them. Warnings about conversions betw
>>  unsigned integers are disabled by default in C++ unless
>>  @option{-Wsign-conversion} is explicitly enabled.
>>
>> +@item -Wreal-conversion
>> +@opindex Wreal-conversion
>> +@opindex Wno-real-conversion
>> +Warn for implicit type conversions from real (@code{double} or @code{float})
>> +to integral values.
>> +
>>  @item -Wno-conversion-null @r{(C++ and Objective-C++ only)}
>>  @opindex Wconversion-null
>>  @opindex Wno-conversion-null
>> Index: testsuite/gcc.dg/Wreal-conversion-1.c
>> ===================================================================
>> --- testsuite/gcc.dg/Wreal-conversion-1.c (revision 0)
>> +++ testsuite/gcc.dg/Wreal-conversion-1.c (revision 0)
>> @@ -0,0 +1,23 @@
>> +// { dg-do compile }
>> +// { dg-options "-Wreal-conversion" }
>> +
>> +#include <stddef.h>
>> +
>> +int func1(int a) {
>> +  double f = a;
>> +  return f;           // { dg-warning "conversion to" }
>> +}
>> +
>> +double func3();
>> +
>> +void func2() {
>> +  double g = 3.2;
>> +  float f;
>> +  int t = g;          // { dg-warning "conversion to" }
>> +  int p;
>> +  p = f;              // { dg-warning "conversion to" }
>> +  func1(g);           // { dg-warning "conversion to" }
>> +  char c = f;         // { dg-warning "conversion to" }
>> +  int q;
>> +  q = func3();        // { dg-warning "conversion to" }
>> +}
>> Index: testsuite/g++.dg/warn/Wreal-conversion-1.C
>> ===================================================================
>> --- testsuite/g++.dg/warn/Wreal-conversion-1.C (revision 0)
>> +++ testsuite/g++.dg/warn/Wreal-conversion-1.C (revision 0)
>> @@ -0,0 +1,24 @@
>> +// { dg-do compile }
>> +// { dg-options "-Wreal-conversion" }
>> +
>> +#include <stddef.h>
>> +
>> +int func1(int a) {
>> +  double f = a;
>> +  return f;           // { dg-warning "conversion to" }
>> +}
>> +
>> +double func3();
>> +
>> +void func2() {
>> +  double g = 3.2;
>> +  float f;
>> +  int t = g;          // { dg-warning "conversion to" }
>> +  bool b = g;
>> +  int p;
>> +  p = f;              // { dg-warning "conversion to" }
>> +  func1(g);           // { dg-warning "conversion to" }
>> +  char c = f;         // { dg-warning "conversion to" }
>> +  int q;
>> +  q = func3();        // { dg-warning "conversion to" }
>> +}
>> Index: c-family/c.opt
>> ===================================================================
>> --- c-family/c.opt (revision 200359)
>> +++ c-family/c.opt (working copy)
>> @@ -677,6 +677,10 @@ Wsign-compare
>>  C ObjC C++ ObjC++ EnabledBy(Wextra)
>>  ;
>>
>> +Wreal-conversion
>> +C ObjC C++ ObjC++ Var(warn_real_conversion) Init(-1) Warning
>> +Warn for implicit type conversions from real to integral values
>> +
>>  Wsign-conversion
>>  C ObjC C++ ObjC++ Var(warn_sign_conversion) LangEnabledBy(C ObjC,Wconversion)
>>  Warn for implicit type conversions between signed and unsigned integers
>> Index: c-family/c-opts.c
>> ===================================================================
>> --- c-family/c-opts.c (revision 200359)
>> +++ c-family/c-opts.c (working copy)
>> @@ -876,6 +876,12 @@ c_common_post_options (const char **pfilename)
>>    if (warn_packed_bitfield_compat == -1)
>>      warn_packed_bitfield_compat = 1;
>>
>> +  /* Enable warning for converting real values to integral values
>> +     when -Wconversion is specified (unless disabled through
>> +     -Wno-real-conversion).  */
>> +  if (warn_real_conversion == -1)
>> +    warn_real_conversion = warn_conversion;
>> +
>>    /* Special format checking options don't work without -Wformat; warn if
>>       they are used.  */
>>    if (!warn_format)
>> Index: c-family/c-common.c
>> ===================================================================
>> --- c-family/c-common.c (revision 200359)
>> +++ c-family/c-common.c (working copy)
>> @@ -2668,7 +2668,7 @@ conversion_warning (tree type, tree expr)
>>    tree expr_type = TREE_TYPE (expr);
>>    location_t loc = EXPR_LOC_OR_HERE (expr);
>>
>> -  if (!warn_conversion && !warn_sign_conversion)
>> +  if (!warn_conversion && !warn_sign_conversion && !warn_real_conversion)
>>      return;
>>
>>    switch (TREE_CODE (expr))
>> @@ -2715,9 +2715,12 @@ conversion_warning (tree type, tree expr)
>>
>>      default: /* 'expr' is not a constant.  */
>>        if (unsafe_conversion_p (type, expr, true))
>> - warning_at (loc, OPT_Wconversion,
>> -    "conversion to %qT from %qT may alter its value",
>> -    type, expr_type);
>> +      {
>> +        int warn_type = (warn_conversion ? OPT_Wconversion :
>> OPT_Wreal_conversion);
>> +        warning_at (loc, warn_type,
>> +                    "conversion to %qT from %qT may alter its value",
>> +                    type, expr_type);
>> +      }
>>      }
>>  }