[prev in list] [next in list] [prev in thread] [next in thread] 

List:       gcc-patches
Subject:    Re: [PATCH] Fix few build warnings with LLVM toolchain
From:       Martin Sebor <msebor () gmail ! com>
Date:       2019-06-28 21:21:27
Message-ID: 930400ff-7a4f-d1ae-a4fe-c63ebff068cf () gmail ! com
[Download RAW message or body]

On 6/28/19 2:10 PM, Segher Boessenkool wrote:
> On Fri, Jun 28, 2019 at 07:46:54PM +0100, Richard Sandiford wrote:
>> Segher Boessenkool <segher@kernel.crashing.org> writes:
>>> On Fri, Jun 28, 2019 at 08:55:00AM -0600, Martin Sebor wrote:
>>>> Jeff reminded me in a code review the other day that GCC does
>>>> have a guideline for defining POD structs with the keyword
>>>> "struct" and classes with ctors/dtors using "class":
>>>>
>>>>    https://gcc.gnu.org/codingconventions.html#Struct_Use
>>>>
>>>> I quickly prototyped a warning to see how closely GCC follows
>>>> this convention.  The result shows that out of just under 800
>>>> structs and classes defined in GCC sources some 200 use
>>>> the keyword 'struct' despite having a ctor or dtor, or about
>>>> 25%.  So as is the case with most other conventions, without
>>>> a tool to help remind us they exist they are unlikely to be
>>>> followed with enough consistency to be worth putting in place
>>>> to begin with.
>>>
>>> The goal is not to have the rules adhered to.  The goal is to have
>>> a more readable / maintainable / etc. codebase.
>>
>> IMO it's a shame we don't follow that attitude for changelogs,
>> where apparently the number of spaces between the name and the email
>> address is of vital importance :-)  Too often a new contributor's
>> first taste of GCC is a slew of comments about things like that.
> 
> Yes.  It is important that people make good enough changelogs though.
> But when a new contributor gets no comments other than about their
> changelog, well, that is less than ideal, right.
> 
>> But surely it's a valid point that we're not following our own
>> conventions on the C++ usage.  I miss how consistent the codebase
>> was in the C days...
> 
> I think you misunderstand me.  We *should* follow our coding conventions.
> My point was that even if we break the rules in a quarter of cases, we
> still win more than we lose, so this doesn't make the rule useless at all.

That may be so in some simple cases (perhaps formatting) but it
doesn't follow in all of them, and certainly not in this one.

The rationale for using "struct" for PODs and "class" for classes
with ctors or a dtor is "to signal more information."  That is only
accomplished if this signaling is consistent.  Otherwise, to be
sure, the user still has to confirm whether a struct really is
a POD by carefully inspecting its definition.

Speaking from recent experience, when I need to know whether I'm
dealing with a POD or not I need a reliable answer.  It does no
good to me if I use a struct thinking it's "most likely" a POD
in a template like hash_map or vec that only work correctly with
PODs and corrupts memory otherwise.  Or if I use a struct thinking
copying it is as fast as memcpy when it actually has a ctor that
dynamically allocates memory, only to find out much later after
benchmarking that it slowed down some important algorithm.  So
I would argue that without strict enforcement, this particular
convention is worse than useless: because of its unreliability
it's misleading and dangerous.

Martin

PS We know today that relying on convention for something as
important as this is a bad idea, and we have reliable ways of
enforcing the basic properties of types in containers and
algorithms: type traits and static assertions (they are
available in a limited form even in C++ 98 and C11).  With
those mechanisms in place these risk can be eliminated.  But
then, even with enforcement, the convention ceases to serve
its intended purpose.
[prev in list] [next in list] [prev in thread] [next in thread]