Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

half.h default user-provided constructor breaks c++ semantics (value/zero initialization vs default initialization) #246

Closed
geotz opened this issue Sep 6, 2017 · 2 comments
Labels
Needs Discussion To be discussed in the technical steering committee

Comments

@geotz
Copy link

geotz commented Sep 6, 2017

The user-provided default constructor for half in half.h does not respect value/zero initialization which makes the type awkward and/or buggy to work with, especially in generic template code that works with other number types (e.g. floats).

Expected behavior (as with float):

using T = float;
std::vector<T> a(5);	// initialized with 0’s
T b[5];			// uninitialized 
T c[5] = {};		// initialized with 0’s
T d;			// uninitialized

Current behavior (half):

using T = half;
std::vector<T> a(5);	// uninitialized
T b[5];			// uninitialized 
T c[5] = {}; 		// uninitialized
T d; 			// uninitialized

Proposed solution (C++11)
In class definition (half.h) define the default constructor as an explicitly defaulted one:

class half
{
  public:

    //-------------
    // Constructors
    //-------------

    half () = default;
// ...

in order to correctly perform value initialization or default initialization (which actually does nothing). The unsigned short member's 0 value coincides with 16-bit floating point value 0. With the proposed solution, we have the same behavior as with floats.

@meshula
Copy link
Contributor

meshula commented Sep 10, 2017

This seems reasonable.

@milasudril
Copy link

I would be happy to see this fix merged soon. This will also make the following compile:

static_assert(std::is_trivially_default_constructible_v<half>);

@meshula meshula self-assigned this Dec 10, 2018
@cary-ilm cary-ilm added the C++ label Jun 13, 2019
@cary-ilm cary-ilm added TODO Needs Discussion To be discussed in the technical steering committee labels Jun 26, 2019
@kdt3rd kdt3rd assigned kdt3rd and unassigned meshula Jul 23, 2019
kdt3rd added a commit to kdt3rd/openexr that referenced this issue Jul 23, 2019
previous cleanup did most of the work, but add an explicit test that
half is now trivial and default constructible

Signed-off-by: Kimball Thurston <[email protected]>
@kdt3rd kdt3rd closed this as completed in 5323c34 Aug 10, 2019
DominicJacksonBFX pushed a commit to boris-fx/mocha-openexr that referenced this issue Jun 22, 2022
previous cleanup did most of the work, but add an explicit test that
half is now trivial and default constructible

Signed-off-by: Kimball Thurston <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Discussion To be discussed in the technical steering committee
Projects
None yet
Development

No branches or pull requests

5 participants