Initializer list constructors cause ambiguity and runtime errors

[johnbartholomew]

Prior to the introduction of the initalizer-list constructors, this code creates an identity quaternion:

glm::quat{1.0f, 0.0f, 0.0f, 0.0f}

After the introduction of the initializer-list constructors, the same code creates a different quaternion (setting x=1 and w=0, whereas before it set w=1, x=0). This does not result in any compile time error or warning, because the code is still correct, it just results in a bug at runtime.

Prior to the introduction of the initializer-list constructors, this code creates an identity matrix:

glm::mat4{1.0f}

After the introduction of the initializer-list constructors, the same code compiles but causes an assertion failure at runtime.

This seems to me to be a step backwards in usability, introducing ambiguity and reducing the effectiveness of compiler type checking in confirming that the API is being used correctly. Note that the uniform initialization syntax is recommended by people such as Herb Sutter for general use (see, Elements of Modern C++ Style); ie, it is not strange to use it by default in C++11 (it's not intended only for initializer lists).

If you guess that I'm writing this issue because I've just been stung by these problems, you are correct.

[Groovounet]

So what would be your recommended resolution here?

[johnbartholomew]

So what would be your recommended resolution here?

That depends on why the initializer-list constructors were added in the first place. It's not obvious to me why they're needed (or useful) here at all, but I'm sure there must have been some reason. If there's a good reason for having those constructors then I guess the only thing you can do about it is to make sure the ambiguity is clearly documented.

[TieJu]

I posted the same issue a minute ago and overlooked this ... :)

Remove the constructors they are not needed, the compiler will use the closest matching constructor for a uniform initialisation list.

[Groovounet]

There is actually a point for the initialized lists. It allows to write code like:

std::vector<glm::mat4> v1{
    {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15},
    {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15}
};

std::vector<glm::mat4> v2{
    {
        { 0, 1, 2, 3 },
        { 4, 5, 6, 7 },
        { 8, 9, 10, 11 },
        { 12, 13, 14, 15 }
    },
    {
        { 0, 1, 2, 3 },
        { 4, 5, 6, 7 },
        { 8, 9, 10, 11 },
        { 12, 13, 14, 15 }
    }
};

Which is pretty nice to me.

The ambiguity on glm::mat4{1.0f} is pretty unfortunate. I need to think about it, a fix would be pretty nice.

[johnbartholomew]

I'm not sure you need initializer-list constructors on glm objects to be able to do that, I think you only need the one on std::vector. See this gist, for example: https://gist.github.com/johnbartholomew/8550473

Edit: I don't know whether it works in the context of the kind of templated constructors that glm uses/would want to use. It's also possible that it relies on some non-standard type deduction behaviour. Further investigation may be needed.

[TiagoRabello]

Yes, @johnbartholomew is actually right.
Initializer-list constructors are not needed if you want to be able to initialize matrix that way, @Groovounet.
Here is a working example of it using only constructors similar to what Matrix4x4 has:
http://ideone.com/ZlfCoZ

[Groovounet]

Well in practice if I disable the initializer_list contructors and compile the core_type_mat4x4.cpp unit tests I get the following errors:

/Users/christophericcio/Documents/Repository/Github/glm/test/core/core_type_mat4x4.cpp:203:12:
No matching constructor for initialization of 'glm::mat4' (aka 'tmat4x4<float, highp>')
/Users/christophericcio/Documents/Repository/Github/glm/test/core/core_type_mat4x4.cpp:215:25: No matching constructor for initialization of 'std::vectorglm::mat4'

/Users/christophericcio/Documents/Repository/Github/glm/test/core/core_type_mat4x4.cpp:230:25: No matching constructor for initialization of 'std::vectorglm::mat4'

/Users/christophericcio/Documents/Repository/Github/glm/test/core/core_type_mat4x4.cpp:235:25: No matching constructor for initialization of 'std::vectorglm::mat4'

This was tested on Apple LLVM version 5.0 (clang-500.2.79) (based on LLVM 3.3svn).

I also tested the behaviour of VC13 and it's very similar.

Thanks,
Christophe

[TieJu]

Same text as for ticket #160:
You need to remove the 'explicit'ness of many constructors that are needed to do the initialization of mat4 and vec4, then it will work.
Explicit is only needed for one parameter constructors to prohibit transformations from one type to an other, like from a single int to a vec or matrix, what is clearly not desired, for multi-param constructors this makes the constructors unusable for such desired transformations.

[Groovounet]

This issue is fixed in GLM 0.9.5 branch for next.

I removed the initializer_list constructors and removed some explicit constructors.

Thanks for contributing,
Christophe