I am so disappointed in you, std::initializer_list
. You are just not what I thought you were.
Lights out
While on the train to Meeting C++ this year, I was working on the lighting subsystem of the 3D renderer for my game abstractanks. Everything was looking fine, until I switched to the release build. Suddenly, my sun light went out. All the smaller lights were still there, it just looked like night instead of day.
Now stuff working in Debug and not working in Release used to be quite common and happens when you’re not correctly initializing built-in variables. So I went digging, but it was not as easy as I had thought. Several hours later, I tracked the problem down to my global light’s uniform buffer initialization code. This is a buffer that is sent to the GPU so the shaders can read all the lighting information. It looked like a fairly innocent for-loop doing byte-copies of matrices and vectors to a buffer:
using Pair = std::pair; auto Mapping = std::initializer_list{ {ShadowMatrix.ptr(), MATRIX_BYTE_SIZE}, {LightDirection.ptr(), VECTOR4_BYTE_SIZE}, {ColorAndAmbient.ptr(), VECTOR4_BYTE_SIZE} }; std::size_t Offset = 0; for (auto const& Each : Mapping) { mUniformBuffer.SetSubData(GL_UNIFORM_BUFFER, Each.second, Offset, Each.first); Offset += Each.second; }
The Culprit
After mistakenly blaming alignment issues for a while, I finally tried looking at the values of Each.second and Each.first. To my surprise, they were bogus. Now what is going on there? It turns out not writing this in almost-always-auto style, i.e. using direct- instead of copy-initialization fixes the problem, so there’s definitely a lifetime issue here.
Looking at the docs, it became apparent that std::initializer_list
is indeed a reference-type that automatically creates a value-type (the backing array) internally and keeps it alive exactly as binding a reference to that array would. For the common cases, i.e. when std::initializer_list
is used as a parameter, this is fine, because the original list lives for the whole function-call expression. For the direct-initialization case, this is also fine, since the reference-like lifetime-extension kicks in. But for copy-initialization, the right-hand-side is done after the std::initializer_list
is copied. So the backing array is destroyed. Oops.
Conclusion and alternatives
Do not use std::initializer_list
unless as a function parameter. It works well for that, and is surprising for everything else. In my case, a naive “extract variable” refactoring of for (auto const& each : {a, b, c}) { /* ... */ }
led me down this rabbit hole.
My current alternative is stupidly simple: a built-in array on the stack:
using Pair = std::pair; Pair Mapping[]{ {ShadowMatrix.ptr(), MATRIX_BYTE_SIZE}, {LightDirection.ptr(), VECTOR4_BYTE_SIZE}, {ColorAndAmbient.ptr(), VECTOR4_BYTE_SIZE} };
It does the same thing as the “correct” version of the std::initializer_list, and if you try to use it AAA-style, at least clang will give you this nice warning: warning: temporary whose address is used as value of local variable 'Mapping' will be destroyed at the end of the full-expression [-Wdangling]
Under Conclusion and Alternatives you warnt to say “do _not_ use initializer_list …”, don’t you?
Of course, you are right. Thanks! Fixed it!
This should not happen in C++17 with guaranteed copy elision. If you are seeing it in C++17 mode, I think it’s a bug in Clang, which seems to be fixed in version 7. See https://godbolt.org/z/bVvT4d
It seems you are right; -Wlifetime agrees! Good find!