Enums in DNA

Archive Blender Development
1

At this moment makesdna doesn’t support enum fields. As a result, the DNA structs have a lot of int, short, and char fields that actually take predefined values from some enumeration. The problem is that it’s often not that clear, at least not at first sight, which values are supposed to go into which field.

Just to name one concrete example, the Object struct in DNA_object_types.h has a field int mode. There is no comment that explains what should go in there, and it’s left as an exercise for the reader to discover there is DNA_object_enums.h with an eObjectMode enum. Ok, the enum itself has a comment /** #Object.mode */ above it, so there is a pointer from the enum to the property that uses it, but not the opposite way. The compiler or an IDE of course has no idea that these two (the int mode field and the enum eObjectMode) are related at all.

I think that using enums more widely in Blender’s code base will make it easier to understand for humans, and also will allow compilers to help us developers more.

Since for C an enum field is just treated as an int, I would suggest to add enum support to makesdna. I don’t know much about makesdna, but as it parses text, I don’t think it should be too hard to just have it interpret enum eSomeEnumType as an alias for int.

@ideasman42, @mont29, what do you think?

5 Likes
2

I really support this idea. Normally when working on an area I first migrate exising defines to an enum so function parameters/fields are described better. Having support in DNA for this is the next step.

Currently some enums are stored as short or char. Perhaps this can be supported by an DNA_ENUM_CHAR_TYPE / DNA_ENUM_SHORT_TYPE what overrides the int alias.

1 Like
4

I also strongly support the idea in principle, although I see two issues.

The C standard does not set hard requirements on which integer types to use for enums. The only requirement it sets is that the integer type must be large enough to represent all enum items. From the C11 N1548 draft:

6.7.2.2 Enumeration specifiers
[…]
Each enumerated type shall be compatible with char, a signed integer type, or an
unsigned integer type. The choice of type is implementation-defined, […] but shall be
capable of representing the values of all the members of the enumeration.

There are probably no cases where we’d have to use something bigger than int to store all possible values of non-bitfield enums. For these it’s likely fine to just reserve 4 bytes in SDNA.
It’s a similar situation as storing integer types through SDNA btw, because the standard also doesn’t define the size of integer types, only the minimum range. So while on 32 bit systems an int is usually 2 bytes wide (IIRC), SDNA always reserves 4 bit for it.

If we wanted to allow enum types in DNA for bitfields too, we’d have to make things more dynamic. It’s not that uncommon to have bitfields wider than 4 byte. So makesdna would have to iterate over the enum values and choose the byte count for the SDNA field based on the largest contained value.

So thinking about this, we could actually get it to work, we just have to be a bit careful at handling field sizes.

However, another issue I see is passing around the enum types outside of DNA. The reason we typically pass around enum values as integer types (rather than the named enum type), is that C doesn’t support forward declaring enum types. E.g. wherever we wanted to do this…:

void foo(eSomeEnumType value);

… the enum definition would have to be in the translation unit, before above’s declaration.
This would require us to have many header files with enum type definitions, so we can include them wherever we pass the types around. This is a bit of an ugly workaround and would have some costs (having to split headers to have minimum headers with needed type definitions, increased compilation time, loosing git history, etc.).
I’m not sure if there’s a good solution for this. C++11 introduced Opaque enum Declarations to workaround this very issue. It’s really a language design limitation.

So overall: Big +1 for the idea. For DNA we could support this, outside of it benefits may not be that big though.

1 Like
5

This would be really nice to support, I’m just not sure it’s practical, it needs investigation.

Since for C an enum field is just treated as an int

This isn’t something guaranteed - although there are some tricks to ensure the enum width, it’s not so nice.

Another down side is if enums are always int’s - we’d wouldn’t want to use them in some cases where the extra memory usage would matter.

Ideally we could specify the underlying size for both C/C++, checking the C2x draft doesn’t mention support for this.

We could support this for C++ and use plain integer types in C, the pre-processor work to support this may be overly complicated though.

6

For me, the most important part of this is the local readability & understandability of the code. I think it would already help a lot if we would use

void foo(short /*eSomeEnumType*/ value);

in cases where we can’t #include the header defining the enum. The same goes for DNA. If we decide that using enums there is too hairy, at least I would suggest adding a comment that declares the enum type of the variable.

Note that this is the opposite of what we’re doing now; we now declare where the enum is used above the enum definition itself. The problem I have with this approach is that it doesn’t directly answer the question “what are valid values for this field?” Sure, you can find it if you search, for example, ‘Object.mode’, but that’s only usable once you know the field is an enum, and once you know such comments are placed at enum definitions. Apart from requiring knowledge instead of being just obvious from the code, this also only applies to struct fields (like Object.mode) and not to function parameters receiving such a mode value. Documenting all uses of the enum at the enum definition doesn’t make sense.

1 Like
7

There’s another thing we could do, which would give some additional benefits:

/* some_header.h */

typedef short eSomeEnumType__Alias;
/* ... */
void foo(eSomeEnumType__Alias value);

That would be done for all headers that miss access to the actual enum declaration. (We’d probably need __Alias2, __Alias3, etc variants in some cases, which I find acceptable.)

We could make this a common pattern, so that it’s easy to extract the actual name out of the alias, and therefore to search for the actual definition.
A benefit is that this works with “Find Occurrences” of IDEs. Even better, it’s much easier to change the actual integer type passed around. E.g. when an enum grows beyond 256 items, we could just change the aliases from char to short in the few typedef's, rather than all over the place.

1 Like
8

We can use the enums in function signatures without any major changes.

  • Split out enums from struct types: DNA_{type}_enums.h
    (Already done for DNA_object_enums.h).
  • Include this header before function declarations.
  • Use the enums in function signatures.

While it helps with readability, it doesn’t help with accidentally mixing up enum types when used with DNA struct members.

3 Likes