This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Fix few build warnings with LLVM toolchain


On 6/28/19 12:46 PM, 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.
:-)  Given that I regularly deal with the random one off contributors
and cut-n-paste their author information from their email which is
always one space I find the adherence to two spaces in the ChangeLog
annoying.  I still try to fix the, but like all the whitespace stuff,
I'd just assume leave that to commit hooks.  There just isn't a lot of
value in having a human looking for whitespace issues.  I'd much rather
be spending time on more substantial questions.


> 
> 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...
Sure, it's a valid point and I fully applaud the effort to add warnings
for things where we can and fix this kind of stuff.

> 
> That can be fixed by changing the conventions if we no longer think
> they're a good idea.  But if someone's willing to change the codebase
> to follow (one of) the coventions instead, and willing to add a warning
> to keep things that way, then that sounds like a really good thing.
> Especially when it's a common convention that others might want a
> warning for too.
Likewise -- I'm certainly open to changing conventions where we don't
think they make sense.  But I'm not going to unilaterally do that.


> 
> And I don't think the current state of the struct/class tags is really
> a result of making the codebase more maintainable.  I get the feeling
> it just kind-of happened.  ISTM we're trying too hard to find a reason
> not to clean this up.
I believe the idea was to distinguish between POD and more complex
types.  POD isn't going to do things behind your back, whereas classes
certainly can (and it's often useful, for example RAII constructs).

Jeff


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]