error: array type has incomplete element type 'uint8_t[] {aka unsigned char[]}'

[Simon]

I am getting this error in MPLAB using the XC8 compiler for AVR (GCC won't work)

It relates to this variable declaration:
uint8_t TCAn_output_pin_enable[] [] = {{0}, { TCA0_waveform_output_0_enable, TCA0_waveform_output_1_enable, TCA0_waveform_output_2_enable }}  ;

I have no idea why, the words used as variable are #define's so will be 1 or 0, the problem seems to be more the 2 dimensional array as all thu similar single dimensional ones work

[DrG]

If you do this uint8_t TCAn_output_pin_enable[] [1].... does the error go away?

I'm thinking (rightly or wrongly) that the compiler can't figure out the size of the elements if you have more than one []

Edit: This has happened to me before and I can't completely remember what I did. This guy is explaining better https://stackoverflow.com/questions/29295152/database-program-array-type-has-incomplete-element-type - but look at the second part of the answer where he notices the error is from element type.

[DavidAlfa]

Exactly, you need to specify the elements in the 1st array dimension.
You can't have undefined number of undefined elements arrays!
But you can have undefined number of 8 element arrays.

[golden_labels]

The previous answers and the SO link indicate why the compiler refused to accept the code and how to make it swallow it.

To explain why it is like that: there is no multi-dimenssional arrays in C. Any array is a sequence of objects of some type T and all T are created equal. You may have an array of int objects, an array of uint_fast16_t objects, an array of struct Something objects. As long as the T is a type. T may also be an array type itself. An array type it T[n], where n is constant. So you may have an array of int[16],  which means it’s an array that itself consits of 16-element arrays of int. T[], however, is not a type — it has no number of elements specified. So you can’t have an array of T[].

There is syntactic sugar that allows you let the compiler determine the number of the elements based on the initializer. But that applies only to the value that is being initialized. The type of each element in that array must be already known beforehead, so you can’t write T[][]. That would require the compiler to recursively guess types, where each entry in the initializer may affect the meaning of each other element in that initializer. It has been decided it would be too complex to require compilers to do so. While this decision may seem weird, consider that what your brain perceives as a multimensional array, a block of L×M×N×… elements, to the compiler is a “one-dimensional array of some type”.

[SiliconWizard]

As said above.

Your declaration actually doesn't make much sense either. How do you think you're going to actually use the elements which are themselves arrays?
I suppose you were expecting to use zero as a sentinel value for showing the end of the list for each inner array?

Guessing - you were maybe expecting:
- the compiler to figure out the maximum number of items required for the "inner" arrays (in your example, that would be 3);
- the compiler to automatically initialize the inner arrays with zero for the missing items in initializers;
- the compiler to be able to give you the size of the inner arrays with something like sizeof(TCAn_output_pin_enable[0]).

This isn't going to happen. C is not that "smart".
Here the solution is just to give the inner arrays a maximum fixed size. Then the 3 points above will work.
Pick a max size for the inner arrays -  for instance 8:

Code: [Select]

uint8_t TCAn_output_pin_enable[] [8] = {{0}, { TCA0_waveform_output_0_enable, TCA0_waveform_output_1_enable, TCA0_waveform_output_2_enable }}  ;

A better approach IMO would be to define a type for the "inner arrays", which are lists of flags as far as I can tell. This type could be an array of uint8_t (which will have to have a fixed size), or a pointer to uint8_t, if you want more efficient use of memory (but saving just a few bytes here is probably not worth it.) Much easier to read than multi-dimensional arrays, and then you can write functions taking the lists as a parameter.

[Simon]

Well I could give it the first index, even the second. Does my declaration in the header file also need the indexes filled in? I guess it does as I still did not get it to work when I put them into the definition in the c file.

The idea was to find a way of writing set up code for a peripheral with multiple instances. The set up code is in a function I call. Rather than give the settings to a function as arguments as there are so many and it just all falls over I am making all settings defines. So in the peripheral header file I have a define for each peripheral register bit. I fill that in almost like a form and then the setup will put those values into the correct register locations.

What I ended up doing which would make the code more compact if it were not optimising out my unnecessary C code (as it never changes during run) is to have further defines that collect up the values of each bit into a register contents define that simply get's assigned to the register address in a function.

The reason I used arrays was so that I can write the functions once with only the peripheral instance as an argument. This variable is used as the array index, so the register value in the define is assigned to the array variable on creation of the array variable, this is then used in the setup function. It sounds convoluted but once written it means that I have a very verbose header file that I can use to change the settings.

[golden_labels]

Code: [Select]

enum ConfigKey {
    ConfigKey_a,
    ConfigKey_c,
    ConfigKey_b,
    // …
   
    ConfigKey_n
};

enum {
    Mask_out1Enable = 1 << 0,
    Mask_out2Enable = 1 << 1,
    Mask_out3Enable = 1 << 2
    // …
};

static unsigned char const config[ConfigKey_n] = {
    [ConfigKey_a] = 0,
    [ConfigKey_b] = Mask_out1Enable | Mask_out2Enable | Mask_out3Enable,
    [ConfigKey_c] = Mask_out1Enable
};

void initialize(enum ConfigKey const key) {
    if (key < 0 || key >= ConfigKey_n) {
        // Handle error
    }
   
    if (config[key] & Mask_out1Enable) {
        // enable output #1
    }
    if (config[key] & Mask_out2Enable) {
        // enable output #2
    }
    if (config[key] & Mask_out3Enable) {
        // enable output #3
    }
   
    // …
}

Explicit size and the indexes in the initializer are there to prevent silly mistakes. With that syntax it is more likely that, if you make an error, either the compiler will scream at you or at least no garbage will be set by the setup function. However, you may skip the ConfigKey part and then you will have automatically sized, yet less safe, array.

[DavidAlfa]

This can be done in a lot of ways. I think the easiest and less trouble-maker way would be using typedefs, passing a structure as argument with all elements already defined. Not undefined number of elements, which can cause out of bounds access if you mess something.

[Simon]

I should probably write it as macro's once per instance. I can then conditionally include them if the enable bit is "1"

[Nominal Animal]

If the array is processed sequentially, and you have at least two reserved values (say NEXT and END), you can use

    const TYPE output_pin_enable[] = {
        /*  0 */ NEXT,
        /*  1 */ 1, 2, NEXT,
        /*  2 */ 3, NEXT,
        END
    };

Note that neither NEXT or END can appear as a value.  To process each enable, you do

    const TYPE *ptr;
    PINNUMTYPE pin;
    for (pin = 0, ptr = output_pin_enable; *ptr != END; ptr++) {
        if (*ptr == NEXT) {
            pin++;
        } else {
            /* Apply (*ptr) to pin (pin). */
        }
    }


If the list is sparse, or you cannot/do not want to use sequential pin numbers, you can do

    const TYPE output_pin_enable[] = {
        1,   1, 2, NEXT,
        5,   3, NEXT,
        END
    };

where the first value (and first value after each NEXT) indicates the element index, and END must be an invalid index (but may appear as a value); only NEXT must be an invalid value.  To process each enable,

    const TYPE *ptr = output_pin_enable;
    while (*ptr != END) {
        const PINNUMTYPE pin = *ptr;
        while (*(++ptr) != NEXT) {
            /* Apply enable (*ptr) to pin (pin). */
        }
    }


To find a specific enable sequence for a pin, you do need to scan through the array, in both above cases.  This means that non-sequential access (access to a specific pin, instead of each pin in the defined order), is relatively slow.

This kind of single-array structure is dense and useful when the accesses are sequential, and you don't need random access to specific pin sequences.  They (Verlet lists) are used in e.g. molecular dynamic simulations, listing the neighbor atom identifiers as a list for each atom, so that interactions that are limited in range do not need to be calculated for all atom pairs/triplets/quads/groups in the system.  It offers a significant speedup for all non-trivial classical-type/non-quantum-mechanical atomic simulations.

This does not rely on any GCC-isms, and I originally saw it used in FORTRAN code.  In Simon's case, I'd consider using the latter form, perhaps guarding some specific lines or line sets with #ifdef FEATURE_OR_PINS ... #endif.

You could even split the pin identifier into two nibbles, high nibble (four bits) identifying the PORT (with END = 0xF0), and low nibble identifying the bit within the port (with NEXT = 0x0F).  Use an AND mask for the PORT (so that 0x00 = PORTA, 0x10 = PORTB, 0x20 = PORTC, and so on) instead of shifting it to low bits in a helper variable, for best efficiency; letting the compiler combine the bit shifts needed to get the correct address for the port control register.  This also means you can define and use macros  for each IO pin, as a single byte then identifies both the port and the pin within the port, with macro names matching whatever you use on the board silkscreen.)