Re: preprocessor directive indentation, cppi

[Jim Meyering]

Bruno Haible wrote:

> Hi Jim,
>
>> Would you mind if I adjust whichever of the 25 in tests/ that are yours ?
>
> Yes, I do mind changing 160 lines of code in just one file because of this.
>
> My point is that 'cppi' wants to enforce a coding style that is pretty far
> away from the current, reasonable coding style in gnulib. It's like activating
> one of the more obnoxious GCC warnings and asking for each occurrence of the
> warning to be fixed.
>
> I do like to indent like this:
>
>    #if A
>    # if B
>    foo
>    # endif
>    #endif
>
> for small to medium sized pieces of code. I do _not_ like to do so for large
> pieces of code when the structure is not complicated. For example, in .h files

I deliberately excluded .h files from a latter proposal because the
case for changing them is weak to nonexistent when the motivation is
a mere double-include guard.

> it is normal to have a double-include guard, and everyone knows this. 
> Similarly
> for test-lock.c. The reason is that I do not want, when adding a #ifdef at the
> top of a file and a #endif at the end, to reindent 160 lines.

Making everything conform to cppi need not be your goal.  However, I
find it useful to adhere to a style that can be mechanically verified.
Thinking about the bigger picture (more than just cpp directives),
I've experimented with using uncrustify to format coreutils.  It is very
configurable, and, more importantly, it is being maintained and it is much
more readable than GNU indent.  However, there are so many configuration
options it can be hard to identify which settings one requires.

> I think this style is reasonable, because it does not enforce the use of
> particular tools or Emacs modes during maintenance, and #if/#endif mismatches
> or related mistakes are rare.
>
> 'cppi' is written to enforce one particular style, and it is different from
> the style I use. Therefore in the current state, 'cppi' is not useful to me.
>
> If 'cppi' would offer several levels of strictness checking, it could become
> useful to me. (Just like GCC offers different warning options, from which a
> programmer can choose.) I have mentioned the kind of warning which I find
> pretty useful:
>   - More than 2 #if / #endif directives in a block without blank lines,
>   - among which at least 1 #if and at least 1 #endif,
>   - and (the entire block not following cppi style indentation,
>          or the number of #ifs and #endif in the block not the same).

If someone improves it, that'd be great,
but I'd prefer to invest in uncrustify.

>> When the distance between #if and matching #endif is too large,
>> it can be a big help to have consistent indentation.
>
> Yes, when the #if structure is complicated, it can be. (vasnprintf.c
> without appropriate indentation would be hopeless.) But when the #ifs are
> simple, like a #if / #else / #endif around a group of functions, the help
> is not big, and I choose in my code not to apply the indentation there.
>
>> Another advantage to religiously-consistent cpp directive indentation
>> is that you can easily write a grep-like tool (yes, I have one somewhere)
>> that will report the nested cpp conditions corresponding to each match.
>
> When I want to understand which kind of code is being actually compiled and
> executed I usually use "cc -E". What is the functionality that your tool can
> bring and that could not be done with other indentation styles?

The moment you use cc -E, you've already chosen some set
of preprocessor symbols to define.  What if the code you're
looking for appears in an #if block that you've compiled away?
What if the code you're looking for appears in two or more
blocks of an #if-#elsif-#elsif-... cascade?
With your approach, you can find them only one at a time, assuming
you enable the guarding cpp expression.  If there are several
matches, I want to find them all right away, and to know what
cpp conditions guard them.

This is not something that happens often in gnulib's code.

In addition, with consistent cpp indentation, the search tool
can search efficiently and simply for containing cpp conditionals.
If cpp directives are not indented consistently, the search tool
would then have to be aware of C comment and string syntax in order
to do its job -- or maybe do something like run cppi on the fly.

>> it lets you distinguish conditional and unconditional
>> #include directives and #define directives based solely
>> on presence or absence of spaces after the "#".
>
> This heuristic does not always work:
>   - sleep.c:# define WIN32_LEAN_AND_MEAN
>     is semantically an unconditional define (it is defined on all platforms
>     where it matters), like in accept.c.

You are applying a different definition of "conditional."
I mean "subject to a cpp conditional".

>   - tsearch.c:#define CHECK_TREE(a) check_tree(a)
>     is a conditional definition. This code comes from glibc.
>
> So in practice, I have to grep for "# *define" anyway.
>
>> > Can cppi be extended (through a command-line option) to flag only
>> > the last case as
>> > improperly indented?
>>
>> Probably, if you're motivated.
>
> I tried to look into it, but found that it's hard to understand and extend the
> logic of a function that's defined by 'flex'. Maybe you have more luck that 
> me;
> I don't use 'flex' much and prefer hand-written C code.

The first version of cppi was in perl, but it had to be able to process
a lot of code very quickly in order to be tolerated on a commit hook.
That was 14 years ago, and that first perl implementation took too long
to process some of the larger source files (containing tens of thousands
of lines of code).  Of course, it had to be robust, which meant parsing
C and C++ comments and strings accurately.  Back then, flex was the
only option.