Bug #8419

Should not require four-space indents on function arg continuation lines

Added by Gordon Ross 30 days ago. Updated 9 days ago.

Status:RejectedStart date:2017-06-22
Priority:NormalDue date:
Assignee:-% Done:

0%

Category:-
Target version:-
Difficulty:Medium Tags:needs-triage

Description

I'm taking issue with the 2nd part of the description in #6459

Additionally, if a function has a very long header, it is split onto
two lines. These continuation lines are supposed to be indented by
four spaces, not a tab. However, cstyle does not verify [ that ].

Note that the old SunOS C-style guide (a copy of which is found here)
https://www.cis.upenn.edu/~lee/06cse480/data/cstyle.ms.pdf
shows both four-space indented forms and tab indented forms
of long function argument declarations. Further, MANY places
in the gate currently use tab-indented function arg continuations,
so this change causes new cstyle complaints in code that's OK.

When do we want four-space indents, and why?

The reason four-space indents are preferred for most continuation
lines is to give them a clearly visible difference from other lines
that might also be indented, i.e. in a long if expression:

        if (this is some long expression
            with the 2nd line indented +4) {
                /* if body indent different from expression indent */
                some_function(args);
        }

There's not so much need for a different indent level with
long function args, because the opening brace (on its own line)
serves to create a sufficiently visible break for the reader, i.e.:

int
some_function(int with_long_args, int on_multiple_lines,
        int like_this)
{
        int var = 1;
        [ ... ]
}


Related issues

Related to illumos gate - Bug #6459: cstyle doesn't detect opening braces on the same line as function header Closed 2015-11-18

History

#1 Updated by Gordon Ross 30 days ago

Proposed fix: Allow either four-space or tab indent for function arg continuation lines.

#2 Updated by Paul Dagnelie 30 days ago

I think I disagree with your premise here; all forms allowed by the style guide are (or should be, there could be bugs) supported by the cstyle changes in 6459. There are 3 ANSI C forms for function definitions. Form 1 isn't relevant, because it only applies when all the arguments fit on one line. Form 2 is the normal 4-space continuation form; note that it explicitly calls for 4-space continuations, rather than being silent and allowing a tab-friendly interpretation. Form 3 is the tab-indented form, but it has the distinction that each argument has a line to itself, which makes it different from your some_function example (call it Form 2b). All three of these forms should be supported by cstyle; I remember testing Form 3 when implementing these changes, though it's possible that I messed up the perl.

According to the style guidelines, Form 2b is in violation and should be tagged by cstyle. If you're arguing that we should deviate from the SunOS style guide, that's a fine discussion to have, though I think we'd need pretty widespread approval for any changes to the style guidelines. Personally I prefer not allowing Form 2b for consistency's sake with other continuation lines, but that's just my opinion.

#3 Updated by Marcel Telka 29 days ago

  • Related to Bug #6459: cstyle doesn't detect opening braces on the same line as function header added

#4 Updated by Toomas Soome 29 days ago

The reason for having so much code not conforming to cstyle is about the fact that we are normally fixing the style as the file is updated. IMO it should be encouraged people to post just pure style updates as well - just as we already have for spelling.

There is also secondary nit - in fact at least freebsd is using the same style (and is having the similar issues about fixing the style) - the [almost?] only difference is about sizeof(). As we do have cross contributions, keeping the style is nice IMO.

Also note that even where tabs are used, it still is huge mess, sometimes there is one tab, then the arg list is aligned to previous line, and then it is just random amount of tabs, so in my view, allowing tab does not really "solve" much.

Of course at the end of the day it is also about the personal taste and preferences - and agreement.

#5 Updated by Gordon Ross 9 days ago

  • Status changed from New to Rejected

It appears I'm in the minority with concerns about these c-style changes.
Closing.

Also available in: Atom