Re: [boost] Variant2 review

Subject: Re: [boost] Variant2 review
From: Niall Douglas (s_sourceforge_at_[hidden])
Date: 2019-04-09 10:38:48

On 09/04/2019 03:58, Gavin Lambert via Boost wrote:
> On 9/04/2019 13:40, Robert Ramey wrote:
>>> Why not use #ifndef JUNIOR_DEVS_DONT_TOUCH or something similarly
>>> never defined (and so never disabling the check) but expressing your
>>> real intent?
>> -1
> 
> I don't particularly agree with that either, but that's what Niall said
> was the real reason for the #ifdef.  I did paraphrase, of course.
> 
> FWIW though I don't think it was intended as a disparagement of junior
> developers in particular.  It's easy enough for anyone, no matter how
> junior or senior, to trip over an assumption in the code -- and in that
> case it's best if the assumption is declared in a fashion that the
> compiler can alert you about, so that you can decide if the violation
> was intentional or not and correct related code accordingly if needed.

Indeed.

Not long ago I broke one of those NDEBUG static asserts in a codebase I
wholly wrote. Even better, around five years ago I had written the comment:

// Hard required to prevent segfault
#ifdef NDEBUG
...

... just before a list of static asserts in a header file.

Now I, for the life of me, could not figure out what I had meant by
that. I spent half a day checking and making sure that I could safely
flip that static assert, and in fact there were no segfaults at risk
there. I had in my head maybe some potential aliasing bug under
optimisation, or something.

In that sense, #ifdef NDEBUG is powerful. It says "be very careful
messing with these asserts, because they're asserting on production
code". Equally, if overused, they can quickly lose their power.

> It's better to have the static_asserts without any #if at all.  (Unless,
> as I said, there's a fundamental requirement for it, eg. with types
> changing size in debug builds.)

For me personally, I may disturb data layout during a set of changes,
which will be developed in debug build. I don't want the static asserts
firing during those changes, nor do I want to have to go through and
disable them all, lest I forget to reenable them.

I do want them to fire when I think I'm finished, and I'm doing final
validation in release build. They usually need adjusting, and sometimes
show I've made a critical mistake. One of the things I particularly
frequently mess up is breaking trivial copyability. Sprinking static
asserts of trivial copyability immediately after class definition is an
*excellent* habit to get into.

Anyway, in general it's a great technique to get in the habit of
deploying. You'll keep a low latency codebase low latency over its
lifetime much easier if you do.

Niall