Issue 5306071: [patch][google] Allow static const floats unless -pedantic is passed.

Issue 5306071: [patch][google] Allow static const floats unless -pedantic is passed. (Closed)

Can't Edit
Can't Publish+Mail
Start Review

Created:
12 years, 6 months ago by Jeffrey Yasskin (google)

Modified:
12 years, 6 months ago

Reviewers:
Diego Novillo

CC:
c-compiler-team_google.com

Base URL:
svn+ssh://gcc.gnu.org/svn/gcc/branches/google/gcc-4_6/

Visibility:
Public.

Description

This patch allows us to migrate to C++11 more incrementally, since we can leave the static const float initializations in place, flip the switch, and then change them to use constexpr. We should NOT forward-port this to any gcc-4.7 branches. gcc/cp/ChangeLog.google-4_6 2011-10-28 Jeffrey Yasskin <jyasskin@google.com> google ref 5514746; backport of r179121 Modified locally to only block static const literals in -pedantic mode. 2011-09-23 Paolo Carlini <paolo.carlini@oracle.com> * decl.c (check_static_variable_definition): Allow in-class initialization of static data member of non-integral type in permissive mode. gcc/testsuite/ChangeLog.google-4_6 2011-10-28 Jeffrey Yasskin <jyasskin@google.com> google ref 5514746; backport of r179121 Modified locally to only block static const literals in -pedantic mode. * g++.dg/cpp0x/constexpr-static8_nonpedantic.C: New. 2011-09-23 Paolo Carlini <paolo.carlini@oracle.com> * g++.dg/cpp0x/constexpr-static8.C: New.

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+54 lines, -2 lines)			Patch
M	gcc/cp/ChangeLog.google-4_6	View	1 2	1 chunk	+14 lines, -0 lines	0 comments	Download
M	gcc/cp/decl.c	View	1 2	1 chunk	+12 lines, -2 lines	0 comments	Download
M	gcc/testsuite/ChangeLog.google-4_6	View	1 2	1 chunk	+14 lines, -0 lines	0 comments	Download
A	gcc/testsuite/g++.dg/cpp0x/constexpr-static8.C	View		1 chunk	+7 lines, -0 lines	0 comments	Download
A	gcc/testsuite/g++.dg/cpp0x/constexpr-static8_nonpedantic.C	View		1 chunk	+7 lines, -0 lines	0 comments	Download

Messages

Total messages: 3

Expand All Messages | Collapse All Messages

Jeffrey Yasskin (google)

This patch is intended for the google/gcc-4_6 branch. Tested with make check-c++ on ubuntu x86-64. ...

12 years, 6 months ago (2011-10-27 16:27:38 UTC) #1

Diego Novillo

On Thu, Oct 27, 2011 at 09:27, <jyasskin@google.com> wrote: > Reviewers: Diego Novillo, > > ...

12 years, 6 months ago (2011-10-28 14:57:40 UTC) #2

On Thu, Oct 27, 2011 at 09:27,  <jyasskin@google.com> wrote:
> Reviewers: Diego Novillo,
>
> Message:
> This patch is intended for the google/gcc-4_6 branch. Tested with make
> check-c++ on ubuntu x86-64.
>
> Should this go to gcc-patches@gcc.gnu.org too, or just the internal
> list?

As you prefer.  Strictly speaking, yes, in case other C++ maintainers
have feedback on your patch.  But given that it is a patch that you
intend to keep in a google release only, then it does not really
matter all that much.

>
> Description:
> This patch allows us to migrate to C++11 more incrementally, since we
> can leave the static const float initializations in place, flip the
> switch, and then change them to use constexpr.
>
> We should NOT forward-port this to any gcc-4.7 branches.
>
> The original patch
> (http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=179121) was by Paolo
> Carlini, but I've changed the intent, so I'm not sure how to credit it
> in the changelog. Guidance is welcome.

When backporting, the ChangeLog entry includes the original ChangeLog
entry (not the SVN rev number).  This way, you both credit the
original change and state the change.

>
> gcc/cp/ChangeLog.google-4_6
> 2011-09-23  Jeffrey Yasskin  <jyasskin@google.com>
>
>       Backport of r179121, modified to s/permerror/pedwarn.
>       * decl.c (check_static_variable_definition): Allow in-class
>       initialization of static data member of non-integral type unless
>       -fpedantic.
>
>
> gcc/testsuite/ChangeLog.google-4_6
> 2011-10-26  Jeffrey Yasskin  <jyasskin@google.com>
>
>       Backport of r179121, modified to allow static const floats.
>       * g++.dg/cpp0x/constexpr-static8.C: New.
>       * g++.dg/cpp0x/constexpr-static8_nonpedantic.C: New.
>
>
>
> Please review this at http://codereview.appspot.com/5306071/
>
> Affected files:
>  M     gcc/cp/ChangeLog.google-4_6
>  M     gcc/cp/decl.c
>  M     gcc/testsuite/ChangeLog.google-4_6
>  A     gcc/testsuite/g++.dg/cpp0x/constexpr-static8.C
>  A     gcc/testsuite/g++.dg/cpp0x/constexpr-static8_nonpedantic.C
>
>
> Index: gcc/testsuite/ChangeLog.google-4_6
> ===================================================================
> --- gcc/testsuite/ChangeLog.google-4_6  (revision 180546)
> +++ gcc/testsuite/ChangeLog.google-4_6  (working copy)
> @@ -1,3 +1,11 @@
> +2011-10-26  Jeffrey Yasskin  <jyasskin@google.com>
> +
> +        google ref 5514746
> +
> +       Backport of r179121, modified to allow static const floats.
> +       * g++.dg/cpp0x/constexpr-static8.C: New.
> +       * g++.dg/cpp0x/constexpr-static8_nonpedantic.C: New.
> +
>  2011-10-20   Diego Novillo  <dnovillo@google.com>
>
>         * gcc.dg/inline_1.c: Remove expected extra errors.
> Index: gcc/testsuite/g++.dg/cpp0x/constexpr-static8.C
> ===================================================================
> --- gcc/testsuite/g++.dg/cpp0x/constexpr-static8.C      (revision 0)
> +++ gcc/testsuite/g++.dg/cpp0x/constexpr-static8.C      (revision 0)
> @@ -0,0 +1,7 @@
> +// PR c++/50258
> +// { dg-options "-std=c++0x -pedantic" }
> +
> +struct Foo {
> +  static const double d = 3.14; // { dg-warning "constexpr" }
> +};
> +const double Foo::d;
> Index: gcc/testsuite/g++.dg/cpp0x/constexpr-static8_nonpedantic.C
> ===================================================================
> --- gcc/testsuite/g++.dg/cpp0x/constexpr-static8_nonpedantic.C  (revision 0)
> +++ gcc/testsuite/g++.dg/cpp0x/constexpr-static8_nonpedantic.C  (revision 0)
> @@ -0,0 +1,7 @@
> +// PR c++/50258
> +// { dg-options "-std=c++0x" }
> +
> +struct Foo {
> +  static const double d = 3.14; // no warning
> +};
> +const double Foo::d;
> Index: gcc/cp/decl.c
> ===================================================================
> --- gcc/cp/decl.c       (revision 180546)
> +++ gcc/cp/decl.c       (working copy)
> @@ -7508,8 +7508,12 @@ check_static_variable_definition (tree decl, tree
>   else if (cxx_dialect >= cxx0x && !INTEGRAL_OR_ENUMERATION_TYPE_P (type))
>     {
>       if (literal_type_p (type))
> -       error ("%<constexpr%> needed for in-class initialization of static "
> -              "data member %q#D of non-integral type", decl);
> +        {
> +          pedwarn (input_location, OPT_pedantic,
> +                   "%<constexpr%> needed for in-class initialization of "
> +                   "static data member %q#D of non-integral type", decl);
> +          return 0;
> +        }

Add a 'FIXME google' here?  Describe why this is different than
upstream.  Helps with merge conflicts.

OK with those changes.


Diego.

Jeffrey Yasskin (google)

12 years, 6 months ago (2011-10-28 20:09:06 UTC) #3

Thanks Diego,

Here's a new version of the patch with fixes for your comments. I'll
submit it in a couple hours unless I hear objections.

On Fri, Oct 28, 2011 at 7:57 AM, Diego Novillo <dnovillo@google.com> wrote:
> On Thu, Oct 27, 2011 at 09:27,  <jyasskin@google.com> wrote:
>> Reviewers: Diego Novillo,
>>
>> Message:
>> This patch is intended for the google/gcc-4_6 branch. Tested with make
>> check-c++ on ubuntu x86-64.
>>
>> Should this go to gcc-patches@gcc.gnu.org too, or just the internal
>> list?
>
> As you prefer.  Strictly speaking, yes, in case other C++ maintainers
> have feedback on your patch.  But given that it is a patch that you
> intend to keep in a google release only, then it does not really
> matter all that much.
>
>>
>> Description:
This patch allows us to migrate to C++11 more incrementally, since we can leave
the static const float initializations in place, flip the switch, and then
change them to use constexpr.

We should NOT forward-port this to any gcc-4.7 branches.


gcc/cp/ChangeLog.google-4_6
2011-10-28  Jeffrey Yasskin  <jyasskin@google.com>

        google ref 5514746; backport of r179121

	Modified locally to only block static const literals in -pedantic
	mode.

	2011-09-23  Paolo Carlini  <paolo.carlini@oracle.com>

		* decl.c (check_static_variable_definition): Allow in-class
		initialization of static data member of non-integral type in
		permissive mode.


gcc/testsuite/ChangeLog.google-4_6
2011-10-28  Jeffrey Yasskin  <jyasskin@google.com>

        google ref 5514746; backport of r179121

	Modified locally to only block static const literals in -pedantic
	mode.

	* g++.dg/cpp0x/constexpr-static8_nonpedantic.C: New.

	2011-09-23  Paolo Carlini  <paolo.carlini@oracle.com>

		* g++.dg/cpp0x/constexpr-static8.C: New.
>>
>> You can review this at http://codereview.appspot.com/5306071/
>>
>> Affected files:
>>  M     gcc/cp/ChangeLog.google-4_6
>>  M     gcc/cp/decl.c
>>  M     gcc/testsuite/ChangeLog.google-4_6
>>  A     gcc/testsuite/g++.dg/cpp0x/constexpr-static8.C
>>  A     gcc/testsuite/g++.dg/cpp0x/constexpr-static8_nonpedantic.C
>>
...
>> Index: gcc/cp/decl.c
>> ===================================================================
>> --- gcc/cp/decl.c       (revision 180546)
>> +++ gcc/cp/decl.c       (working copy)
>> @@ -7508,8 +7508,12 @@ check_static_variable_definition (tree decl, tree
>>   else if (cxx_dialect >= cxx0x && !INTEGRAL_OR_ENUMERATION_TYPE_P (type))
>>     {
>>       if (literal_type_p (type))
>> -       error ("%<constexpr%> needed for in-class initialization of static "
>> -              "data member %q#D of non-integral type", decl);
>> +        {
>> +          pedwarn (input_location, OPT_pedantic,
>> +                   "%<constexpr%> needed for in-class initialization of "
>> +                   "static data member %q#D of non-integral type", decl);
>> +          return 0;
>> +        }
>
> Add a 'FIXME google' here?  Describe why this is different than
> upstream.  Helps with merge conflicts.

Done.

> OK with those changes.
>
>
> Diego.
>

Expand All Messages | Collapse All Messages