[PATCH] extend scope of t/codingstd/c_indent.t

[plobsing]

t/codingstd/c_indent.t currently only scans C code when the C preprocessor stack isn't empty (ie between #if and #endif).

Much of parrot's C code doesn't fall into that category and is currently unchecked and sometimes incorrectly indented (eg: src/ops/sys.ops, which uses a lot of 2-space indenting).

The attached patch tweaks t/codingstd/c_indent.t so that it no longer ignores such code.

[plobsing]

c_indent.t.patch

[jkeenan]

Replying to plobsing:

The attached patch tweaks t/codingstd/c_indent.t so that it no longer ignores such code.

I applied your patch and then ran prove -v t/codingstd/c_indent.t. It now reports thousands of violation of the c_indent coding standard. (See attachment.) Is that what you expected?

Thank you very much.

kid51

[jkeenan]

Output of c_indent.t after applying plobsing's diff

[plobsing]

Replying to jkeenan:

I applied your patch and then ran prove -v t/codingstd/c_indent.t. It now reports thousands of violation of the c_indent coding standard. (See attachment.) Is that what you expected?

Yes. And looking at a (relatively small) sampling, I have yet to see any false-positives.

I can make a patch (or set of patches) to fix these. But the number of affected lines makes me think it might be hard to review.

I am open to suggestions.

[coke]

Is 0 an invalid indent level?

Also, this code:

    for (edge = unit->bb_list[loop_info->header]->pred_list;
         edge;
         edge = edge->pred_next) {

is lined up vertically to be slightly more aesthetically pleasing. A strict reading of the standard probably disallows this, but I think the alternative (in the general case) is probably a little less readable.

(This is not a vote, just an observation.)

[plobsing]

Replying to coke:

Is 0 an invalid indent level?

The heuristic that's being triggered for the code with 0-space indentation is roughly "the first line of a top level block must have exactly 4-spaces of indentation".

Most of these can be made to go away if we modify this to be the first non-comment line another dubious case is eliminated by ignoring yacc/lex %{ ... %} blocks as top level blocks.

[plobsing]

more permissive c_indent.t patch

[jkeenan]

Replying to plobsing: The first patch gave these aggregated results:

#   Failed test 'Correctly indented C files'
#   at t/codingstd/c_indent.t line 227.
# incorrect indenting in C file found 1573 occurrences in 111 files:

The second patch gave these:

#   Failed test 'Correctly indented C files'
#   at t/codingstd/c_indent.t line 229.
# incorrect indenting in C file found 1548 occurrences in 92 files:

There is a very practical problem here: If we were to accept this patch, who would we get to correct these 1548 errors?

(I am not going to do that :-).)

kid51

[plobsing]

Replying to jkeenan:

There is a very practical problem here: If we were to accept this patch, who would we get to correct these 1548 errors? (I am not going to do that :-).) kid51

I understand that it isn't really reasonable to expect someone else to fix this. I will (try to) fix the errors if/when I get a commit bit.

[plobsing]

I've tried to get this to work in a sane fashion. I have failed. Closing ticket.