← Back to team overview

kicad-developers team mailing list archive

Re: [PATCH] Fix m_materials init-to-0

 

In this case, if you did not trust the default construction of
glm::vec3f, you could provide a user-defined ctor to SMATERIAL. But in
this case, that's not required.

Quick note, on closer inspection, KiCad sets GLM_FORCE_CTOR_INIT, so
we will actually be calling a user-defined ctor (which does the
init-to-0.0f manually), rather than the default of value-initing an
aggregate (with a ctor, it's no longer aggregate). The effect is the
same, but the generated code seems rather longer!

>From C++20, you will be able to do GNU-style { .foo = 2,, .bar = 3}
init as standard too.

Cheers,

John
On Fri, Nov 9, 2018 at 1:40 PM Mário Luzeiro <mrluzeiro@xxxxx> wrote:
>
> Thanks, it's clear as mud!
> I guess next time I will take as option to do a proper initialization element by element in a WYSIWYG fashion ;)
>
> ________________________________________
> From: John Beard <john.j.beard@xxxxxxxxx>
> Sent: 09 November 2018 13:20:38
> To: Mário Luzeiro; Kicad Developers
> Subject: Re: [Kicad-developers] [PATCH] Fix m_materials init-to-0
>
> Hi Mário,
>
> In C++11, this is list-initialisation, which has various behaviours
> depending on what you are initialising. In this case:
>
> * m_materials is class type with default ctor, so value-init it.
> Value-init of aggregates with {} does aggregate-init. This does
> list-init of each member with {}:
>     * SMATERIAL is the same, so list-init each member with {}:
>         * glm::vec3f's is the same (explicit "= default" in the hpp
> file), so list-init each member with {}:
>             * 3 floats are value-inited (-> 0.0f, in whatever the
> implementation's floating point type is, most likely IEEE-754)
>         * floats are value-inited (as above)
>
> In C++14, this is *slightly* different in that aggregate-init comes
> before value-init for class type with default ctors. So the step goes
> right from list-init to aggregate-init (skipping the value-init). The
> result is the same.
>
> Pre-C++11, there was no list-initialisation, so this would just be
> aggregate-init, which would do value-init directly.
>
> Clear as mud, right?
>
> FYI, memset-to-0 for floating point is unsafe/non-portable in C too
> (unless you can guarantee that the format is IEEE-754, but that's not
> in the standard): 6.2.6  Representations of types, 1.1: "The
> representations of all types are unspecified except as stated in this
> subclause."
>
> Cheers,
>
> John
> On Fri, Nov 9, 2018 at 12:28 PM Mário Luzeiro <mrluzeiro@xxxxx> wrote:
> >
> > Hi John,
> >
> > Thanks for proper fix that code. That illustrate how much "C programmer" I was at the time..
> > Is the "= {}" initialization available on all C++ versions?
> >
> > Mario Luzeiro
> >
> > ________________________________________
> > From: Kicad-developers <kicad-developers-bounces+mrluzeiro=ua.pt@xxxxxxxxxxxxxxxxxxx> on behalf of John Beard <john.j.beard@xxxxxxxxx>
> > Sent: 09 November 2018 12:20:01
> > To: Kicad Developers
> > Subject: [Kicad-developers] [PATCH] Fix m_materials init-to-0
> >
> > Hi,
> >
> > Simple patch to fix a use of memset to reset an array of floats and
> > glm::vec3f's to 0.
> >
> > This is unsafe, not least as float types have implementation-defined
> > value representations [1]. Memory-0 = float-0 is true for IEEE-754,
> > and glm::vec3f happens to not have any book-keeping data that could be
> > smashed by this (if m_materials contained a std::string or
> > std::vector, this would be very bad karma), so it does happen to work.
> >
> > Cheers,
> >
> > John
> >
> > [1]: http://www.open-std.org/jtc1/sc22/open/n2356/basic.html#basic.fundamental


Follow ups

References