Re: [bug #59270] [PATCH] node.h: avoid C++11 feature (non-const init in class)

[Ingo Schwarze]

Hi Branden,

G. Branden Robinson wrote on Thu, Oct 15, 2020 at 11:34:28PM +1100:
> At 2020-10-14T15:45:16+0200, Ingo Schwarze wrote:

>> this is the second portability issue that killed the build for me.
>> Again, i'm looking for an OK to push the fix.
>> 
>> Since i have little experience with C++, please check that the fix is
>> correct and matches your intent.

> TL;DR: LGTM, +1.

Pushed, thanks for checking.

> As far as I understand C++[1], yes it does.  I have an allergy to
> allowing even the potential of a read from uninitialized storage, but I
> recognize that this was not part of the ethic of early C++, where
> everything was about "zero-cost" abstraction a.k.a. G01NG F4573RRRRRR.

I surfed the web a bit before writing the patch to understand the
background, and i got the impression that Stroustrup's reason for
disallowing this particular feature was not so much speed but rather
simplicity of the compiler and linker, arguing that the compiling
and linking infrastructure needed for allowing it would be excessively
complex and fragile, making bugs more likely in the infrastructure.

Of course, nowadays, few people working on compilers worry about
complexity, so i'm not all that surprised that it was allowed in a
later version of the standard.

On the other hand, trying to always initialize storage when defining it
in application code is one possible attempt at making bugs in application
programs less likely.  I'm not quite sure how good that attempt is
because initializing storage before you have a reasonable value can
also cause or hide programming errors.  So that feels like a double-
edged sword.

Either way, this didn't feel like a feature of sufficient
importance to start requiring C++11...

>> I don't think a Changelog entry is needed here, either.
>> This mini-issue never made it into a release and no user-visible
>> change is intended.

> Okay.  Per the rough consensus hammered out 3 years ago or so, my
> personal practice is to add a ChangeLog entry if:
> 
> 1. I'm changing executable code;
> 2. I'm fixing a ticket;
> 3. I'm making a "big" change to documentation.
> 
> That way the ChangeLog doesn't get clogged with my numerous editorial
> revisions to documentation[2].
> 
> So my ChangeLogging threshold is lower than yours for executable code,
> but I don't mind that.  I'm still the new kid around here and I
> appreciate the increased probability of code reviews.

Makes sense.  Then again, this one is only amending a recent commit
of yours in a minor way, so i consider it covered by your recent
ChangeLog entry.

I don't think every ticket necessarily needs a ChangeLog entry
though.  Since some patches that were sent out in the past were
forgotten (i also forgot about some), i now tend to open a ticket
for every patch where i ask for an OK, even for minor patches.
Finding uncommitted, forgotten patches lying around months later
is just too annoying and disruptive to let it happen...

Yours,
  Ingo