Marc-André Lureau <marcandre.lur...@gmail.com> writes: > Hi > > On Fri, Jun 22, 2018 at 10:08 AM, Markus Armbruster <arm...@redhat.com> wrote: >> Subject: explicitly >> >> Marc-André Lureau <marcandre.lur...@redhat.com> writes: >> >>> The C standard has the initial value at 0 and the subsequent values >>> incremented by 1. No need to set this explicitely. >>> >>> This will prevent from artificial "gaps" when compiling out some enum >>> values and having unnecessarily large MAX values & enums arrays. >>> >>> Signed-off-by: Marc-André Lureau <marcandre.lur...@redhat.com> >>> --- >>> scripts/qapi/common.py | 7 ++----- >>> 1 file changed, 2 insertions(+), 5 deletions(-) >>> >>> diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py >>> index 60c1d0a783..68a567f53f 100644 >>> --- a/scripts/qapi/common.py >>> +++ b/scripts/qapi/common.py >>> @@ -2032,14 +2032,11 @@ typedef enum %(c_name)s { >>> ''', >>> c_name=c_name(name)) >>> >>> - i = 0 >>> for value in enum_values: >>> ret += mcgen(''' >>> - %(c_enum)s = %(i)d, >>> + %(c_enum)s, >>> ''', >>> - c_enum=c_enum_const(name, value, prefix), >>> - i=i) >>> - i += 1 >>> + c_enum=c_enum_const(name, value, prefix)) >>> >>> ret += mcgen(''' >>> } %(c_name)s; >> >> What excactly in your series depends on this? >> >> What safeguards do you propose to ensure an enumeration with >> conditionals is compiled only with the exact same conditionals within >> the same program? >> >> Example of the kind of deathtrap to guard against: compile >> >> typedef enum Color { >> COLOR_WHITE, >> #if defined(NEED_CPU_H) >> #if defined(TARGET_S390X) >> COLOR_BLUE, >> #endif /* defined(TARGET_S390X) */ >> #endif /* defined(NEED_CPU_H) */ >> COLOR_BLACK, >> } Color; >> >> in s390x-code (COLOR_BLACK = 2) and in target-independent code >> (COLOR_BLACK = 1), then linking the two together. > > This was a trick used in previous iterations. Now that we have a > separate target schema and generated code/headers, and since we have > poisoned identifiers, this should never happen. > >> Yes, I know a similar deathtrap will be set up for struct and union >> types. No excuse for ignoring either of the two. > > Having gaps in the enums makes it harder to iterate over all values, > and we use more memory than necessary when allocating based on MAX > value. > > It's not a big problem, but I consider this more important than this > artificially made up broken build example.
Made up yes, but the making up comes from painful experience debugging real programs :) I'm not at all opposed to "contracting" enums. I just want to be persuaded it's safe. I think your argument is "Now that we have a separate target schema and generated code/headers, and since we have poisoned identifiers, this should never happen." Can you elaborate? If that goes as I expect it to go, the next request will be "now put that into your commit message" :)