This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Fix -Wundef for FEATURE_INDEX_1.


> This is the simplest patch I can come up with to fix FEATURE_INDEX_1
> -Wundef warnings. The constant definition is duplicated because we
> can't use the enum constant in assembly nor in the other macros.

What does "nor in the other macros" mean?  Of course a macro can be
based on arithmetic plus use of another constant that is usable in the
contexts where the macro is used.  enum constants cannot be used in
assembly, nor in #if contexts.  Did you mean to say that the other
macros are used in #if contexts and therefore must not use enum
constants?

> The down side is the slight duplication. I considered adding another
> macro e.g.
> #define __FEATURE_INDEX_1 0
> ...
> # define FEATURE_INDEX_1 __FEATURE_INDEX_1
> ...
> However, that just seemed ugly, so I left the duplication.

When "duplication" concretely means two instances of one macro whose
value is zero, then it seems less ugly than many other things.  But in
the general case, duplication is worse than most other things.

In this situation, the question is, what are we getting from
FEATURE_INDEX_* being enum constants rather than macros at all?

This is internal-only code, so ease of maintenance is really the only
issue that we care much about.  The general reason to have enum
constants for things is so that you can use them in the debugger rather
than either looking up values in the source by hand or relying on -g3
macro info.  That is far less useful than the general case when its an
anonymous enum, so there is no type to which you can cast an integer
value to get the symbolic name trivially in gdb.  Hence, literally the
only benefit is typing FEATURE_INDEX_1 in gdb.  If we cared about that,
we'd make all the macros in init-arch.h be enum constants.  So I think
we just aren't getting anything worthwhile from having it be an enum.
Let's eliminate the duplication by just having one unconditional #define.


Thanks,
Roland


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]