Using std::array to implement variadic construction of class with list-of-self as subclass

Question

I have a situation with a base class for whom a container of itself is a subclass. So a Block of Item has various ways in which it acts like an Item itself. I want to be able to initialize Block as Block {foo, baz, bar...} in a variadic syntax.

Here are the twists:

• When a Block is constructed, the parameters in the initialization list are not simply Items. (Imagine some represent two items... while others might just be a different meaning for the literal types passed in.) To simplify the example I am suggesting that constructing an Item from a C-string literal is not available to the user... but that a Block initialization list would know what to do with that in context.

• I'm trying to avoid creating std::vector in the course of the initialization, and instead building a std::array for as near-zero runtime overhead as I can get. Each element in the Block initialization should be constructed (or copy constructed) just once in the process; right in place in the array... though this array is only needed temporarily. As far as I can tell, this rules out std::initializer_list and I must use a variadic function.

The basic idea is to create a class that has containership of an Item which is a friend of Item... here called Listable. It has its own set of constructors:

class Item {
 friend class Listable; 
 friend class Block;
 Item (char const *) { std::cout << "Istring\n"; }
 Item (void *, size_t) { std::cout << "Iblock\n"; }
public:
 Item (Item const &) { std::cout << "Icopy\n"; }
 Item (float) { std::cout << "Ifloat\n"; }
};
class Listable {
 Item item;
public:
 Listable (Item const & i) : item (i) { std::cout << "Litem\n"; }
 Listable (const char * s) : item (s) { std::cout << "Lstring\n"; }
 Listable (float f) : item (f) { std::cout << "Lfloat\n"; }
};
class Block : public Item {
protected:
 Block (Listable * l, size_t c) : Item (l, c) { std::cout << "Bpointer\n"; }
 template<size_t N> 
 Block (std::array<Listable, N> a) : Block (&a[0], N) { std::cout << "Barray\n"; } 
public:
 template<typename... Ts>
 Block (Ts const & ...t) : Block (std::array<Listable, sizeof...(t)>{t...})
 { std::cout << "B\n"; }
};

Here is a usage, showing a string not working for the construction of an Item while the Block knows how to handle it. The goodItem is the only one that needs to be copied, as its original instance was not constructed in-place in an array slot:

#include <iostream>
#include <array>
/* #include the classes above */
int main() {
 auto goodItem = Item {10.20};
 auto goodBlock = Block {goodItem, "blue", "red", 3.04};
 /* auto misplacedItem = Item {"purple"}; */ // errors, correctly...
}

I compiled with no warnings with:

g++ -pedantic -Wall -Wsign-conversion -Wextra -Wcast-align -Wcast-qual -Wctor-dtor-privacy -Wdisabled-optimization -Wformat=2 -Winit-self -Wlogical-op -Wmissing-declarations -Wmissing-include-dirs -Wnoexcept -Wold-style-cast -Woverloaded-virtual -Wredundant-decls -Wsign-promo -Wstrict-null-sentinel -Wstrict-overflow=5 -Wswitch-default -Wundef -Werror -Wno-unused --std=c++11 test.cpp -o test

This output is what I wanted to see:

Ifloat
Icopy
Litem
Istring
Lstring
Istring
Lstring
Ifloat
Lfloat
Iblock
Bpointer
Barray
B

Can anyone spots any problems or suggestions for improvement on this technique?

• Morwenn: Sorry, but once answered, you should not modify a question's code, especially if it might invalidat the answers (it was the case here). See what you can and cannot do after receiving an answer.
• HostileFork says dont trust SE: @Morwenn I understand if you don't cite where it came from and if there weren't history. But history offers a tool for that. The problem in code review (especially if you are trying to establish a best practice to explain something) is that the artifact becomes less useful. Asking people to review your code and review the review becomes double work. (I suppose your answer is "don't put the codereview link in your documentation/etc. then"...which is all right, but it makes it a less useful resource.) Seems answers could just cite the version snapshot they commented on.
• Morwenn: Well, I didn't make the rules, but it's easier for future readers to read the original then read the answers than having to check the history whenever they want to understand the answers.
• HostileFork says dont trust SE: @Morwenn There's perhaps some sensible compromise, because the desire (for me) is to have a minimal-complete-verifiable-example and a record of the discussion (along with opportunity for more discussion)...with the ability to test that minimal example (via copy-paste)...linked from the (much more complicated version) in source. I guess a link at the top to a Gist or other offsite resource of "last best state--read this first" with all the rest kept for history can work.
• Morwenn: Well, it's a discussion you will have to bring to meta then.

Answer 1

I have a few things to say about your code:

• Do you intend to actually do things in the constructors instead of just displaying strings? Knowing this would help the review process. Currently, you do not store anything passed to Item, but if you intend to store things at some point, it may change things.

• Instead of using &a[0] which is kind of obscure and not easy to find with a simple search query, you shoud use a.data() which clearly shows the intent.

• It would probably be a good idea to pass the std::array by const reference in the constructor instead of passing it by copy. That will avoid useless copies:

  template<size_t N> 
  Block (std::array<Listable, N> const& a) : Block (&a[0], N) { cout << "Barray\n"; }
  

  Of course, that implies that you take const Listable* instead of Listable* at some point (but we come back to my first point: we lack information about what your code is supposed to do). I don't know what you intend to do with this, but passing a pointer to the underlying memory of a temporary doesn't feel right. This may create dangling pointers.

  EDIT: since your code isn't supposed to store the pointer, then taking the std::array by copy shoud be safe. But as you noted in the comments, using an rvalue-reference parameters should be even better: it binds to the temporary, you can modify it and no copy is performed. Seems like the best solution.

• HostileFork says dont trust SE: Thanks for the tip on .data, didn't know about that. This is for a binding to an interpreter through a C function that wants an array of Listable that it uses temporarily and writes to; in some cases the results of the writes are used (here we discard them, so a mutable temporary is all right). const & seems to work but a regular reference complains...since there was only one call to Icopy I assumed the array was being passed by reference. I've never needed to use std::array before so it's new to me; is there any way to properly pass it by mutable reference?
• HostileFork says dont trust SE: Ah, Rvalue reference works, since it's a temporary. Is that safe enough if the array only has to live for the duration of the constructor call? (And yes, Item does store things...in particular a native struct type known to the bound-to interpreter. If that changes anything.)
• Morwenn: @HostileFork So the C function simply uses the parameters and does not store them? Then you're right, keeping a temporary std::array parameter is fine. The rvalue-reference constructor seems fine too.