diff mbox

Make declaration of wide_int_storage match its definition

Message ID alpine.LSU.2.11.1407052036320.30120@tuna.site
State New
Headers show

Commit Message

Gerald Pfeifer July 5, 2014, 6:42 p.m. UTC
Farther below in wide-int.h, we define wide_int_storage as a class:

  class GTY(()) wide_int_storage
  {
  private:
    HOST_WIDE_INT val[WIDE_INT_MAX_ELTS];
    unsigned int len;
    unsigned int precision;
  :

The patch below, which I applied as obvious after a full bootstrap on
i386-unknown-freebsd10.0, makes the declaration match the definition.

Gerald

2014-07-05  Gerald Pfeifer  <gerald@pfeifer.com>

	* wide-int.h (wide_int_storage): Change declaration from struct 
	to class.

Comments

Andrew Pinski July 5, 2014, 7:03 p.m. UTC | #1
> On Jul 5, 2014, at 11:42 AM, Gerald Pfeifer <gerald@pfeifer.com> wrote:
> 
> Farther below in wide-int.h, we define wide_int_storage as a class:
> 
>  class GTY(()) wide_int_storage
>  {
>  private:
>    HOST_WIDE_INT val[WIDE_INT_MAX_ELTS];
>    unsigned int len;
>    unsigned int precision;
>  :
> 
> The patch below, which I applied as obvious after a full bootstrap on
> i386-unknown-freebsd10.0, makes the declaration match the definition.

This patch is fine but c++ allows class and struct be used interchangeable. If there is a compilers which does not it is broken and should be report to them.  Yes that means clang is broken. 

Thanks,
Andrew

> 
> Gerald
> 
> 2014-07-05  Gerald Pfeifer  <gerald@pfeifer.com>
> 
>    * wide-int.h (wide_int_storage): Change declaration from struct 
>    to class.
> 
> Index: wide-int.h
> ===================================================================
> --- wide-int.h    (revision 212304)
> +++ wide-int.h    (working copy)
> @@ -284,7 +284,7 @@
> 
> template <typename T> struct generic_wide_int;
> template <int N> struct fixed_wide_int_storage;
> -struct wide_int_storage;
> +class wide_int_storage;
> 
> /* An N-bit integer.  Until we can use typedef templates, use this instead.  */
> #define FIXED_WIDE_INT(N) \
Gerald Pfeifer July 5, 2014, 7:25 p.m. UTC | #2
On Sat, 5 Jul 2014, pinskia@gmail.com wrote:
> This patch is fine but c++ allows class and struct be used 
> interchangeable. If there is a compilers which does not it is broken 
> and should be report to them.  Yes that means clang is broken.

Clang does allow for it (it actually is the stage 1 compiler on 
FreeBSD 10, the platform I used for my tests); it just warns about 
it.  About 400 times.

That was just one, factor, though.  Declarations being consistent
with definitions stroke me as A Good Thing[tm] per se. :-)

Gerald
Jakub Jelinek July 5, 2014, 7:29 p.m. UTC | #3
On Sat, Jul 05, 2014 at 09:25:50PM +0200, Gerald Pfeifer wrote:
> On Sat, 5 Jul 2014, pinskia@gmail.com wrote:
> > This patch is fine but c++ allows class and struct be used 
> > interchangeable. If there is a compilers which does not it is broken 
> > and should be report to them.  Yes that means clang is broken.
> 
> Clang does allow for it (it actually is the stage 1 compiler on 
> FreeBSD 10, the platform I used for my tests); it just warns about 
> it.  About 400 times.

That doesn't change anything that the warning is very much broken.
In C++ struct is simply a class with default public:, class with a default
private:, when you are just forward declaring it, whether it defaults to
public: or private: doesn't matter at all, therefore the warning just
enforces some weirdo clang coding style.

	Jakub
Trevor Saunders July 6, 2014, 7:39 p.m. UTC | #4
On Sat, Jul 05, 2014 at 09:29:31PM +0200, Jakub Jelinek wrote:
> On Sat, Jul 05, 2014 at 09:25:50PM +0200, Gerald Pfeifer wrote:
> > On Sat, 5 Jul 2014, pinskia@gmail.com wrote:
> > > This patch is fine but c++ allows class and struct be used 
> > > interchangeable. If there is a compilers which does not it is broken 
> > > and should be report to them.  Yes that means clang is broken.
> > 
> > Clang does allow for it (it actually is the stage 1 compiler on 
> > FreeBSD 10, the platform I used for my tests); it just warns about 
> > it.  About 400 times.
> 
> That doesn't change anything that the warning is very much broken.
> In C++ struct is simply a class with default public:, class with a default
> private:, when you are just forward declaring it, whether it defaults to
> public: or private: doesn't matter at all, therefore the warning just
> enforces some weirdo clang coding style.

I thought the warning was to help people who care about MSVC and its
utterly broken mangling, but I also agree its a pretty silly warning
that I wouldn't mind turning off.

Trev

> 
> 	Jakub
Mike Stump July 7, 2014, 10:51 a.m. UTC | #5
On Jul 5, 2014, at 11:42 AM, Gerald Pfeifer <gerald@pfeifer.com> wrote:
> Farther below in wide-int.h, we define wide_int_storage as a class:
> 
>  class GTY(()) wide_int_storage
>  {
>  private:
>    HOST_WIDE_INT val[WIDE_INT_MAX_ELTS];
>    unsigned int len;
>    unsigned int precision;
>  :
> 
> The patch below, which I applied as obvious

Thanks.  We did want them to match.
diff mbox

Patch

Index: wide-int.h
===================================================================
--- wide-int.h	(revision 212304)
+++ wide-int.h	(working copy)
@@ -284,7 +284,7 @@ 
 
 template <typename T> struct generic_wide_int;
 template <int N> struct fixed_wide_int_storage;
-struct wide_int_storage;
+class wide_int_storage;
 
 /* An N-bit integer.  Until we can use typedef templates, use this instead.  */
 #define FIXED_WIDE_INT(N) \