5

I have an array of (pointers to) arrays of different lengths, which I learned I could define using compound literals:

const uint8_t *const minutes[] = {
    (const uint8_t[]) {END},
    (const uint8_t[]) {1, 2, 3, 4, 5 END},
    (const uint8_t[]) {8, 9, END},
    (const uint8_t[]) {10, 11, 12, END},
    ...
};

gcc accepts this just fine, but clang says: pointer is initialized by a temporary array, which will be destroyed at the end of the full-expression. What does this mean? The code seems to be working, but then again, lots of things seem to work when they point to memory that's no longer allocated. Is this something I need to worry about? (Ultimately I only really need it to work with gcc.)

Update: Something fishy is going on. It says here that:

Compound literals yield lvalues. This means that you can take the address of a compound literal, which is the address of the unnamed object declared by the compound literal. As long as the compound literal does not have a const-qualified type, you can use the pointer to modify it.

   `struct POINT *p;
   p = &(struct POINT) {1, 1};

This example code seems to be doing exactly what I'm trying to do: a pointer to something defined by a compound literal. So is the clang error message legitimate? Will this end up pointing to unallocated memory when compiled with either clang or gcc?

Update 2: Found some documentation: "In C, a compound literal designates an unnamed object with static or automatic storage duration. In C++, a compound literal designates a temporary object, which only lives until the end of its full-expression." So it seems that clang is right to warn about this, and gcc probably ought to as well, but doesn't, even with -Wall -Wextra.

I can't guess why a useful C feature was removed from C++, and no elegant alternative way of accomplishing the same thing was provided.

Improve this question
7
  • Are you compiling with -W flag when using gcc ? It might show up if you are not Commented Jul 3, 2015 at 17:56
  • Yep. -Wall -Wextra, which is apparently the same thing. Commented Jul 3, 2015 at 18:05
  • have you tried removing the unnecessary (const uint8_t[]) at the beginning of each row? Commented Jul 3, 2015 at 18:11
  • 1
    @RichardHodges it's not unnecessary. That's the syntax for compound literals. Without it the compiler expects something that looks like a pointer, and instead sees something in curly braces and complains: error: braces around scalar initializer for type 'const uint8_t* const Commented Jul 3, 2015 at 18:20
  • In C, this is legal at file scope, not legal at block scope. Can you please edit your question to show the scope . In C++ compound literals are not allowed at all . Commented Jul 5, 2015 at 7:00

3 Answers 3

Reset to default
3

Well, clang is right, and this shall be done that way:

namespace elements
{
    const uint8_t row1[] = {END};
    const uint8_t row2[] = {1, 2, 3, 4, 5, END};
    ...
}
const uint8_t *const minutes[] = {
    elements::row1,
    elements::row2,
    ...
};

You can think of more C++ solution, like using std::tuple:

#include <tuple>

constexpr auto minutes = std::make_tuple(
    std::make_tuple(), 
    std::make_tuple(1,2,3,4,5),
    std::make_tuple(8,9,10)); 

#include <iostream>
#include <type_traits>
int main() {
    std::cout << std::tuple_size<decltype(minutes)>::value << std::endl;
    std::cout << std::tuple_size<std::remove_reference_t<decltype(std::get<1>(minutes))>>::value << std::endl;
}
Improve this answer
Sign up to request clarification or add additional context in comments.

5 Comments

Why I don't like your first option: it's messy. It's prone to typos, and If I re-order the rows or add a row later, there's lots of variable renaming that has to be done, or just leaving the row numbers out-of-order. Having said that, it does seem to be the most straightforward way to get the right in-memory representation of the data.
I'll have to look into your second option, with std::tuple and constexpr. I'm more comfortable with C than C++ (if you haven't guessed - but I'm using some C++ libraries for this project), and I'm coding for an embedded system with tiny memory, so I don't want to use any feature if I don't know exactly what's going on behind the curtain. (Yes, I realize the design philosophy behind C++, or OOP in general, is that you're not supposed to need to know the internal implementation, but... :)
@Josh If you afraid of wrong ordering - you can use BOOST_PP_ENUM - see boost.org/doc/libs/1_39_0/libs/preprocessor/doc/ref/enum.html.
@Josh When talking about future insertions, deletions - you can use old trick from [basic] times - use tens numbers initially (row10,row20, ...) Then you can easily add row15 at any time...
yeah, that's exactly the "ugly" I was talking about :) Anyway, In my case the data isn't going to change, and I don't even need the namespace because this data structure is defined in a separate file and the names of the inner parts won't have external linkage. So it's not going to be a maintenance nightmare... it's just inelegant.
1

Well that means, this expression

(const uint8_t[]) {1, 2, 3, 4, 5 END},

creates a temporary object — temporary because it doesn't have any name which can last beyond the expression of which it is a part — which gets destroyed at the end of the full expression, which means this:

 };

defines "the full expression", at which point all the temporary objects will get destroyed, and the array of pointers minutes holds pointers which point to destroyed objects, which is why the compiler is giving warning.

Hope that helps.

Improve this answer

8 Comments

Yes, I suspected as much. So what do I do about it? :)
Define them as array first. Or better redesign it. Try use std::array.
Can you give an example of what you mean? There's got to be a way to do this that doesn't require creating dozens of individually-named variables that I don't actually want or intend to use... right? This compound literals method was one I got from an answer here on SO - is it actually wrong?
(This is for an embedded system, with 30k program storage and 2k ram, so I need the code to be as tiny, efficient, simple, and generally under-my-control as possible. I'm trying to avoid anything "fancy" like std::array.)
Can you define (const uint8_t[]) {1, 2, 3, 4, 5 END} as an array? Like const uint8_t a0[] = {1, 2, 3, 4, 5 END}; and so on. And then use them as elements of minutes.
|
1

update: thanks to deniss for pointing out the flaw in the original solution.

static constexpr uint8_t END = 0xff;

template<uint8_t...x>
const uint8_t* make()
{
    static const uint8_t _[] = { x..., END };
    return _;
}

const uint8_t* const array[] = {
    make<>(),
    make<1, 2, 3, 4, 5>(),
    make<8, 9>(),
    make<10, 11, 12>()
};
Improve this answer

5 Comments

This compiles, although it makes the compiled app bigger, and makes it take a bit more memory - not ideal for an embedded system. Any idea why that is? I'm also not sure what it's doing - as I understand it a static variable should allocate space once that persists between calls to the function. So how is this function allocating new space each time it's called?
It's generating one static function per different row of numbers. Each static function will contain a static array (and a hidden mutex). When you say more memory, how much are we talking? After optimisation a few hundred bytes might be worth the trade off for ease of maintenance.
Note: for the same number of parameters, this function generates only 1 array (filled with values from the first invocation).
Yup that's true. That's an oversight of mine.
@MattMcNabb hah! yay! :-) I use _ in these functors and lambdas like this because it seems reasonably easy to me to interpret it as 'it' or 'the relevant thing'.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.