← Back to team overview

kicad-developers team mailing list archive

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

 

Hey John,

I merged your patch.  Thanks for the technical explanation as well.

Cheers,

Wayne

On 11/9/2018 8:53 AM, John Beard wrote:
> 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
> 
> _______________________________________________
> Mailing list: https://launchpad.net/~kicad-developers
> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~kicad-developers
> More help   : https://help.launchpad.net/ListHelp
> 


References