Re: Intel icc finds too many errors in guile 1.8.8 to compile

[Neil Jerram]

address@hidden (Ludovic Courtès) writes:

> Hi,

Hi Ludo,

Thanks for your comments below.  I should have posted this change for
review first, so apologies for not doing that.

> Neil Jerram <address@hidden> writes:
>
>> "I.Gutheil" <address@hidden> writes:
>
> [...]
>
>>> debug.c(62): error #188: enumerated type mixed with another type
>>>     scm_dynwind_begin (0);
>>>
>>> This could be "corrected" by declaring
>>> in dynwind.h
>>> SCM_API void scm_dynwind_begin (int) instead of the derived datatype
>>> which could only be 1
>>> and in dynwind.c
>>>
>>> void
>>> scm_dynwind_begin (int flags)
>>> ...
>>
>> Agreed; using enums here is just asking for trouble.  I've changed it to
>> #defines.
>
> Per C99 Section 6.4.4.3, “an identifier declared as an enumeration
> constant has type int”, so ICC is pedantic.

Maybe yes.  But on the other hand (and arguing from common sense instead
of from the standards) there surely has to be some point - in terms of
allowing the compiler to spot mistakes - of using an enum instead of
plain constants?

> I think having disjoint ‘scm_t_wind_flags’ and ‘scm_t_dynwind_flags’
> types was helpful to indicate which flags are valid for which function.
> Thus, I’d rather have fixed it by adding a named enumeration constant
> for 0 in each of these types.

The use of an enum for combinable bit flags is actually new to me, so I
don't know what is generally supported.  But it seems logical to me that
if you have

  enum flag_t {
    flag_a = 1,
    flag_b = 2,
    flag_c = 3
  };

and

  void my_func(flag_t flags);

and the compiler then complains about

  my_func(0);

surely it would equally complain about any combination of more than one
flag, such as

  my_func(flag_a | flag_b);

The reasoning being that

- an enum converts to an int when used in some context that doesn't
  handle enum directly; so `flag_a | flag_b' is an int

- my_func expects a flag_t, not an int.

So, in summary, does the use of enum for bit flags really make overall
sense?

> At any rate, I think we should probably keep “typedef int
> scm_t_wind_flags;” and “typedef int scm_t_dynwind_flags”.

Yes, good idea.  And with that I could revert a lot of my change, maybe
including the doc changes.

> What do you think?

It may be that I've misunderstood how enums can be used for flags, so
please let me know what you think about my argument above.  If enums can
be used portably here, we can make a more minimal change.  If not, we
should still keep the typedefs, and revert the bits of my change that we
can.

Regards,
        Neil