Is it legal to initialize an array via a functor which takes the array itself as a parameter by reference?
Asked Answered
G

3

13

In the question Idiom for initializing an std::array using a generator function taking the index?,which basically asks how one can initialize an array of an arbitrary type which is not necessarily default constructible, I came up with the following (highly unorthodox) solution.

#include <cstddef>
#include <utility>
#include <array>
#include <iostream>

struct Int{
    int v;
    Int(int v):v{v}{}
};

int main()
{
    auto gen = [](size_t i) { return  Int(11*(i+1)); };
    std::array<Int, 500000> arr = [&arr, &gen](){
        for(std::size_t i=0; i < arr.size(); i++)
            new (arr.data() + i) Int(gen(i));
        return arr;
    }();
    for(auto i : arr) { std::cout << i.v << ' ';}
    std::cout << '\n';
}

In the solution, a functor (in this case a lambda, but I'm interested in the general case) is used to initialize the array. The functor takes the array to be initialized by reference, constructs the non-default-constructible elements via placement new, and then returns the array.

I am not entirely sure about whether this is really legitimate or not. GCC, Clang and MSVC all seems to suggest this is valid. For GCC and Clang I have also turned on the sanitizer so that undefined behaviors can be detected. The access of arr.size() seems fine as it is just a compile-time constant. The use of arr.data() also seems fine because the lifetime of the array arr starts after the = in std::array<int, 500000> arr= and arr has a well-defined address, which should just be what arr.data() returns because arr is an aggregate, but I'm not entirely sure. I'm also not sure about whether the placement new is valid from the standard perspective. For the arr = [&arr,&gen]{...; return arr;} I am also not sure whether the new rvalue semantics in C++17 is necessary to make the snippet valid, or whether it is also legitimate in earlier C++ standards (e.g. C++14).

So the question is, is it legitimate to access the to-be-initialized array this way in a functor that is used to initialize itself, and why?


For additional context, I have read answers to the question that inspired this question (linked above) and the suggested duplicates there. The answers there basically boils down to one of two things:

  • Generate a local array in the lambda and returning it. This is helpful in the case where the initialization has difficulty due to const, but not in the case where the type is not default constructible.
  • Using some sort of variant based on std::index_sequence. This suffers from the implementation limit of templates and won't work for large array sizes.

Based on these, I believe the solution here has its practical value unless someone can come up with a solution that does not suffer from the above two limitations, and this is not just a question of theoretical interest.

Gribble answered 26/6, 2024 at 14:23 Comment(15)
#78673164 You can take a ref but not write inside before the end of initialization. RVO may be your friend: you don't need to capture the array, just create a local one and return it (be careful use RVO which is mandatory, not NRVO). I think that this post answer might help also.Swam
@Swam Thank you, that answers part of my question. The thing is I cannot create a local one because the type is not default constructible. See the linked question that inspired this question for detailsGribble
See also: How come I can capture an object being constructed in the lambda doing the construction?Slipover
There is nothing "wrong" here. This is default construction of array<int,50000> called arr. Followed by calling a function (functor) that writes to the fully constructed arr, said functions returns arr... Making a copy in the process. THen it is assigned to arr. Does it work. yes. Does it work as expected. yes. Good idea: probably not. The assign serves no purpose.Jemine
@Jemine Re "the assign serves no purpose": How else would you initialize a large array of type T where T is not default-constructible? Re "This is default construction of array<int,50000> called arr" Definitely not.Gribble
@Jemine std::array has no default constructor, its an aggregate type. Also there is not assignment. T name = something is initialization, not assignment.Sanburn
the in-place construction is bad. array constructor already constructed everything. In this case, it'll work, but on more complex elements... probably not.Jemine
@Jemine The array does not have a constructor, and std::array<Int, 500000> arr; doesn't compile. I don't know how I can make it more clear to you the importance that Int is not default constructible.Gribble
Be aware, that this does call a copy-constructor of the array, and performs an actual copy of all Int objects, even if the address of the objects used in the copy-constructor are equal (meaning: address of the copy-from argument equals this). May be insignificant for this particular class Int, but I presume you have something more complex in actual code.Gumma
Would it be possible to use a fold expression to form a braced initializer list for the array initialization? foonathan.net/2020/05/fold-tricks The expression to expand would look like (++n,Int(n))Swam
@Swam No. It suffers from the same limitation as std::index_sequence-based methods.Gribble
IMHO your questions should be slightly rephrased. The interesting issue is not about the legality of the construct but how to do the init with your 2 constraints: std::index_sequence limitation and non default constructibility of the elements.Swam
@Swam I thought the answer to that question should be posted to the linked question instead. If you think it is sufficiently unique I may consider asking another question phrased in the way you suggest. If you found a solution and decide to post as an answer there I'll certainly upvote it.Gribble
indeed you're right, I see that in the link you provided that both constraints have been given. I don't think another question is neededSwam
Takeaway from this: at the point where you find yourself saying you’ll write a new class to wrap the buffer of uninitialized storage to manually call the destructor on each element when the generator function returns, you might even be better off just wrapping an array in a class that you can construct from a generator.Consecrate
H
12

This is UB, and doesn't work correctly for most types.

The use of arr.data() also seems fine because the lifetime of the array arr starts after the = in std::array<int, 500000> arr=

No, the lifetime starts when the initialization is complete. After = the name merely becomes visible. This makes arr.data() UB.

Returning arr will call a copy constructor of std::array with itself as the argument (because of mandatory copy elision), which will do the same for every element, which is not something type authors ever expect (e.g. adding std::string x; as a data member to Int causes a segfault for me).

You can confirm this by adding logging to the copy constructor:

Int(const Int &x) {std::cout << this << ' ' << &x << '\n';}

For each element, this prints the same address twice.


Pre-C++17, when there was no mandatory copy elision, another possible behavior was for arr to be copied into a temporary by return, which is then moved back into arr. That move uses a move constructor rather than move assignment, which overwrites existing elements without calling their destructors, which is also problematic.

Handgrip answered 26/6, 2024 at 16:7 Comment(14)
"adding std::string x; as a data member to Int causes a segfault for me". This is convincing enough. Sadly that original question is closed as duplicate. I'm still looking for a way to create a large array of non-default-constructible type then.Gribble
proof (?) of UB... if the functor or anything inside it throws. Then the destructors of objects created so far will not be cleaned up.Jemine
I don't think you can solve this with std::array because you're expecting aggregation on a nonaggrgatable object. You could make your own std::array-like container which has a byte array (T*sizeof(T)) then use placement new and delete properly (you'll have to keep track where construction of your own class succeeded. so you can cleanup correctly.Jemine
@WeijunZhou Just use std::vector. If you don't want that size to be changeable make it a const std::vector.Sanburn
@Sanburn That would be equivalent to a const std::array<Int, N>, not std::array<Int, N>. The elements should still be re-assignable.Gribble
Why do you mention RVO in your answer? The copy constructor will simply launch by normal semantic. The only remark I see is that there is no RVO/NRVO because arr is captured and not local variable.Gumma
@Gumma I meant that the temporary created by returning by value is not copied/moved to arr, but rather merged with it. I guess "RVO" isn't the right word, this is mandatory copy elision from C++17.Handgrip
"mandatory copy elision". That means things are different in C++14? Note the question is dual-version tagged.Gribble
@WeijunZhou I think in C++14 and earlier, if the compiler decides to not elide, arr can be copied into the temporary, which is then moved back to it. But that move is move-construction rather than assignment, so existing elements get overwritten without their destructors being called, which is also bad.Handgrip
I see. You can edit that into the answer.Gribble
Will do tomorrow.Handgrip
What temporary? There is no temporary arr, everything is being done in-place over the main's arr captured by the lambda. That's why RVO/copy-elision does not apply and a full copy is performed upon return.Gumma
@Gumma Returning by value creates a temporary, unless it's elided.Handgrip
Can you pinpoint the rule that says that the lifetime starts after initialization in this case (@Gumma also): std::array is implicit lifetime, thus doen't its lifetime start as soon as storage is allocated for it? As an automatic storage duration variable, isn't its storage already available at its point of declaration? (I'm probably wrong but can you explain me why?) thanks in advance.Swam
G
7

As HolyBlackCat points out, formally arr.data() is UB. In practice, this is going to be equal to address of arr itself and I can't think of any actual, reasonable implementation where it would fail.

The bigger issue (also pointed out by HolyBlackCat) is the fact that arr is being returned which triggers a copy constructor to initialize arr itself again. This leads to invocation of copy constructor of Int with itself as an argument.

Int::Int(const Int& other) {
    // &other == this
}

that is something that no one expects. While it may be benign to Int it may cause some real, harmful undefined behavior with other types implemented by someone else. Consider for example:

#include <array>
#include <iostream>
#include <memory>

int construct = 0;
int destruct = 0;

struct Foo{
    Foo() { ++construct; }
    ~Foo() { ++destruct; }
};
using FooPtr = std::shared_ptr<Foo>;

int main() {
    {
        auto gen = []() { return std::make_shared<Foo>(); };
        std::array<FooPtr, 5> arr = [&arr, &gen](){
            for(std::size_t i=0; i < arr.size(); i++)
                new (arr.data() + i) FooPtr(gen());
            return arr;
        }();
    } // arr dies here
    std::cout << "constructs=" << construct << " destructs=" << destruct << "\n";
}

This actually gives me

constructs=5 destructs=0

with all three major compilers (clang, gcc, msvc)

godbolt example

Gumma answered 26/6, 2024 at 16:51 Comment(3)
Related: https://mcmap.net/q/561615/-check-for-quot-self-assignment-quot-in-copy-constructor/3966456Gribble
(Not allowed to comment on upvotes? We'll see about that...): +1 for the useful example with the counters.Slipover
The risk with UB isn't necessarily that an "actual, reasonable implementation" would fail, but that the compiler would conclude that it's impossible and do something entertaining (for suitable definition of "entertaining") when it optimizes.Hofmannsthal
C
0

As an alternative, even if the element type is not default-constructible, you can still initialize a buffer of uninitialized memory, and create a std::array from that using std::to_array and a pointer cast.

Be warned: The elements of the temporary array are not destructed after being moved from! Moving leaves most types in a trivially-destructible state, but this might not be true in every case.

Sample code:

#include <array>
#include <memory> // uninitialized_move
#include <ranges>
#include <utility>

using std::size_t;

struct Int{
    int v;
    constexpr Int(int v) noexcept : v{v} {}
};

constexpr size_t array_len = 24;

std::array<Int, array_len> generate_array() noexcept {
    // Generator for the elements of the returned array:
    constexpr auto source = std::ranges::transform_view(
        std::ranges::iota_view(0, static_cast<int>(array_len)),
        [](const int i)constexpr{return Int(11*(i+1));}
    );
    // Buffer of uninitialized memory:
    alignas(Int) std::array<char, sizeof(Int)*array_len> scratch;
 
    std::uninitialized_move_n(
        source.begin(),
        array_len,
        reinterpret_cast<Int*>(scratch.data())
    );
    return std::to_array<Int, array_len>(std::move(*reinterpret_cast<Int(*)[array_len]>(scratch.data())));
}

#include <cstdio>
#include <iostream>

int main() {
    static const auto xs = generate_array();
    for (const Int& x : xs) {
        std::cout << x.v << ' ';
    }
    std::cout << '\n';
    return EXIT_SUCCESS;
}

The current version of MSVC on the Compiler Explorer lacks std::uninitialized_move_n, but Microsoft does support it. As a workaround, you might fall back on std::uninitialized_move.

You could also use the generator above to construct a std::vector and copy your std::array from that. For large arrays, which might cause an (ahem) stack overflow, this has the advantage that it constructs the temporary copy on the heap.

#include <array>
#include <cassert>
#include <ranges>
#include <utility>
#include <vector>

using std::size_t;

struct Int{
    int v;
    constexpr Int(int v) noexcept : v{v} {}
};

constexpr size_t array_len = 24;

std::array<Int, array_len> generate_array() { // Can now throw bad_alloc.
    // Generator for the elements of the returned array:
    constexpr auto source = std::ranges::transform_view(
        std::ranges::iota_view(0, static_cast<int>(array_len)),
        [](const int i)constexpr{return Int(11*(i+1));}
    );
    std::vector<Int> scratch(source.begin(), source.end());
    assert(scratch.size() == array_len);

    return std::to_array<Int, array_len>(std::move(*reinterpret_cast<Int(*)[array_len]>(scratch.data())));
}

#include <cstdio>
#include <iostream>

int main() {
    static const auto xs = generate_array();
    for (const Int& x : xs) {
        std::cout << x.v << ' ';
    }
    std::cout << '\n';
    return EXIT_SUCCESS;
}

Try it on the Godbolt compiler explorer.

Update

As of 2024, a more-useful, but potentially non-portable, solution for large arrays is to bypass std::to_array and directly reinterpret_cast the buffer as a pointer to a std::array. This causes the return object to be move-constructed, which for a large array of this type calls memcpy. For whatever reason, if you use std::to_array, some compilers will spend an excessive amount of time and memory doing template metaprogramming on a large array.

Although the Standard effectively guarantees that a built-in array T[N], a range of pointers from the start to the end of the array, and the range [ data(), data()+size() ) of a std::array or std::vector have the same value representation, the first element of an array is not formally layout-convertible to an array. There is also no requirement that a std::array have the same address as its data(), or that it not have a stricter required alignment. So, this works on all the most-used compilers, but try it at your own risk. I do, however, put in a static_assert that the address of a std::array<T, 1> is the same as its data, as a sanity check against the most-likely way this could break.

Therefore, this version works on the compiler explorer at sizes where the more-portable ones break down.

#include <array>
#include <cassert>
#include <ranges>
#include <utility>
#include <vector>

using std::size_t;

struct Int{
    int v;
    constexpr Int(int v) noexcept : v{v} {}
};

// C++20 does not guarantee that a std::array is layout-compatible with a built-in array, so:
static constexpr std::array<Int,1> layout_check = {Int(0)};
static_assert(static_cast<const void*>(std::addressof(layout_check)) == static_cast<const void*>(layout_check.data()), "");

constexpr size_t array_len = 500'000;

std::array<Int, array_len> generate_array() noexcept {
    // Generator for the elements of the returned array:
    constexpr auto source = std::ranges::transform_view(
        std::ranges::iota_view(0, static_cast<int>(array_len)),
        [](const int i)constexpr{return Int(11*(i+1));}
    );
    // Buffer of uninitialized memory:
    alignas(Int) std::array<char, sizeof(Int)*array_len> scratch;
 
    std::uninitialized_move_n(
        source.begin(),
        array_len,
        reinterpret_cast<Int*>(scratch.data())
    );
    return std::move(*reinterpret_cast<std::array<Int, array_len>*>(scratch.data()));
}

#include <cstdio>
#include <iostream>

int main() {
    static const auto xs = generate_array();
    for (size_t i = 0; i < xs.size(); i += 1000) {
        std::cout << xs[i].v << ' ';
    }
    std::cout << '\n';
    return EXIT_SUCCESS;
}

Although you likely want to create large arrays on the heap:

#include <array>
#include <cassert>
#include <ranges>
#include <utility>
#include <vector>

using std::size_t;

struct Int{
    int v;
    constexpr Int(int v) noexcept : v{v} {}
};

constexpr size_t array_len = 500'000;

// C++20 does not guarantee that a std::array is layout-compatible with a built-in array, so:
static constexpr std::array<Int,1> layout_check = {Int(0)};
static_assert(static_cast<const void*>(std::addressof(layout_check)) == static_cast<const void*>(layout_check.data()), "");

std::array<Int, array_len> generate_array() { // Can throw bad_alloc.
    // Generator for the elements of the returned array:
    constexpr auto source = std::ranges::transform_view(
        std::ranges::iota_view(0, static_cast<int>(array_len)),
        [](const int i)constexpr{return Int(11*(i+1));}
    );
    std::vector<Int> scratch(source.begin(), source.end());
    assert(scratch.size() == array_len);

    return std::move(*reinterpret_cast<std::array<Int, array_len>*>(scratch.data()));
}

#include <cstdio>
#include <iostream>

int main() {
    static const auto xs = generate_array();
    for (size_t i = 0; i < xs.size(); i += 1000) {
        std::cout << xs[i].v << ' ';
    }
    std::cout << '\n';
    return EXIT_SUCCESS;
}

Consecrate answered 27/6, 2024 at 9:49 Comment(24)
std::to_array from clang seems to have the same issue with std::index_sequence based methods and doesn't work for large N. Upvoted anyway.Gribble
@WeijunZhou I just compiled with an array of 500,000 Int, and the only problem I had was the compile time. The implementation limit might be a limitation of the Compiler Explorer sandbox, although it might also be a library issue (since I wasn’t on Linux) You could also try -stdlib=libstdc++.Consecrate
For the first snippet, can we use a raw array as the buffer and reinterpret_cast it directly to std::array? I suppose it is well defined because std::array itself should have implicit lifetime. This avoids the issue whether the implementation uses std::index_sequence to implement to_array.Gribble
@WeijunZhou Yes; see my edit.Consecrate
I could be wrong but I think the buffer may need to be raw array because implicit object creation does not cover the case of std::array<char, N> as the bufferGribble
And if it is indeed valid for implicit objevt creation in a char array subobject instead of a complete object, then I'll just wrap the char array in some custom wrapper and make use of RAII by calling std::destroy in the destructor. It should then fix the issue you mentioned in the "Be Warned" part.Gribble
@WeijunZhou Even if you consider “object” to be a magic word, std::uninitialized_move explicitly creates “objects,” so it’s fine. You might also create your buffer as an array of std::aligned_storage<Size, Alignment>::type, which is deprecated but does explicitly say it’s suitable to use as uninitialized storage for an object.Consecrate
@WeijunZhou Thinking about it some more, it would probably be safer to use uninitialized_move_n and code defensively around a potential buffer overrun.Consecrate
I agree. If we focus on generate_array(), you may also move the temp char array to the heap to avoid blowing up the stack.Gribble
@WeijunZhou Already have a version that does.Consecrate
Oh you mean the vector version. I am not entirely sure you can reinterpret_cast part of an array of non-char type with a different size (determined by the capacity of the vector) like that, but that may be worth its own language-lawyer question. I think in practice it would work.Gribble
@WeijunZhou I don’t see anything in N4680 that says the ``reinterpret_cast<std::array<T,N>*>` is legal. An implementation of std::array would be able to add other arbitrary gunk at the beginning, or a checksum at the end, for example, so long as data() == addressof(front()). There would be more guarantees if we could move the data directly to result.data(), but that would require us to have a valid std::array, and we can’t default-construct one.Consecrate
@WeijunZhou The std::vector should be exactly the same size, and we could test that at runtime, to be safe.Consecrate
@WeijunZhou Made a number of fixes and added some assertions. Maybe my downvoter will even come back for another look.Consecrate
I'm still not convinced you can reinterpret_cast a T* that you obtain from vector::data() to a pointer to T[N], let alone std::array<T,N>, given that: 1. It is not even legal to cast the first element of T[N] to the whole array (see this) and 2. Formally vector never really creates an array object in the first place. Like I said this is mostly language-lawyering. I may stick to using a new array expression with unsigned char type myself and use IOC.Gribble
@WeijunZhou Language-lawyering: [vector.data] specifically says that [data(), data() + size()) is contiguous and “a valid range.” The [[iterator.concept.contiguous] requirement “provides a guarantee that the denoted elements are stored contiguously in memory,” An array also has contiguous storage, and [expr.add] guarantees that pointer and array arithmetic have the same layout. However, basic.compound says, “An array object and its first element are not pointer-interconvertible, even though they have the same address.” So, the reinterpret_cast is not guaranteed to work.Consecrate
Yes, that's what I'm thinking. However using an unsigned char array buffer (either from the stack or the heap) and then reinterpret_casting to a std::array is guaranteed to work due to implicit object creation. So I guess that's a better way in terms of language-lawyering.Gribble
@WeijunZhou Which section of the Standard are you referring to? I don’t believe a std::array is required to have the same address as its first element, or an alignment restriction on its data that is no stronger. I do a static_assert to test for the first condition, but the second is a possible incompatibility, let’s say if every std::array aligns its data to a cache-line boundary and uses aligned load/store on an architecture that traps if those are misaligned.Consecrate
I think you can use reinterpret_cast to start the lifetime of the std::array object (call it pArr), but not its individual elements, just like it is (explicitly) valid to start the lifetime of a raw array without starting the lifetime of individual elements. (This is something I'm not sure about and worth arguing with) Then it should be well-defined to access pArr->data() and start from there. The alignment issue can be handled by providing an alignas() when you allocate the unsigned char buffer.Gribble
@WeijunZhou Or a fat-pointer implementation might, at runtime, refuse to convert a pointer to an array unless the pointer has a size associated with it that says the conversion is safe from causing a buffer overrun. (Although the data() member of a contiguous container likely will, even then.)Consecrate
That is a valid concern. However given that the array type is explicitly called out by the standard to be an implicit lifetime type I see no reason why std::array cannot behave the same. Like I said if that is valid I will use pArr->data() to loop for the elements, then it is no longer a concern whether pArr == pArr->data().Gribble
@WeijunZhou I think we’re in agreement, then, that implementations are allowed to behave the way all major compilers do, and allow the cast to work, but they could also break it, and I can even think of some sane reasons to. Another thing i could be doing is aligning the buffer to at least the alignment of a std::array<T,N>, if greater than that of T.Consecrate
Mostly. What I meant was to change reinterpret_cast<Int*>(scratch.data()) to reinterpret_cast<std::array<Int,N>*>(scratch.data())->data() in the first variant (sorry about the abomination). As for the alignment, yes you can try to improve on that and see if it does anything about the downvote.Gribble
Let us continue this discussion in chat.Consecrate

© 2022 - 2025 — McMap. All rights reserved.