Code formatting of C2's c++

[lerno]

This might seem like a minor issue, but wading through thousands of lines of Clang (I've cut away in excess of 30,000 lines of code), there are two coding habits that makes the code IMMENSELY hard to read.

One is single line if's:

Code: [Select]

if (foo)
  do_something();

This obviously is even worse when adding else as well. The problem here is when you try get an overview of  a function. Here K&R + single statement on two lines is making something really horrid:

Code: [Select]

namspace foo {

/* many lines of code */

  void a_method() {
  /* MANY lines of code */
    do_struff();
    if (foo)
      do_something();
  } // <- What ends here??

/* more lines of code */

} // <- What ends here???

Basically it's hard what block ends where.

I prefer (1) single statements ifs ALWAYS on a single line, Allman style. Any of those would solve the problem, BOTH is best:

ALLMAN

Code: [Select]

namspace foo
{

/* many lines of code */

  void a_method()
  {
  /* MANY lines of code */
    do_struff();
    if (foo)
      do_something();
  }

/* more lines of code */

}


K&R Single line:

Code: [Select]

namspace foo {

/* many lines of code */

  void a_method() {
  /* MANY lines of code */
    do_struff();
    if (foo) do_something();
  }

/* more lines of code */

}

ALLMAN single line:

Code: [Select]

namspace foo
{

/* many lines of code */

  void a_method()
  {
  /* MANY lines of code */
    do_struff();
    if (foo) do_something();
  }

/* more lines of code */

}

I understand most have a preference for K&R, so I'm not going to fight for Allman. But that two line single statement is a killer.

Switch-case where "case" does not have indention is difficult for similar reasons, although the two line if is by far the worst.

[bas]

I played around with clang-format and that seems to be a good candidate. I'll look at an initial style and propose that..

There is a .clang-format file in the root of the archive. It can be applied by running

Code: [Select]

clang-format-i main.cpp
Please use that on several files and check whether it does something really nasty or bad style-wise

[lerno]

It looks like spaces went from 3 -> 4? 

[lerno]

We can certainly use 4, but I'd just mention that this is different from how the current code looks that uses 4.

- Row breaks comments. I don't know if this is a good idea.

- Is it possible to require { } on if-else where each is a single statement?

Eg:

Code: [Select]

if (Res.isUsable()) Cases.push_back(Res.get());
else return StmtError();

// =>

if (Res.isUsable()) {
  Cases.push_back(Res.get());
} else {
  return StmtError();
}

Note that single line ifs are fine! Just not anything multi-line should have { }

Sometimes linebreak is weird:

Code: [Select]

    StmtResult Res = Actions.ActOnDeclaration(id->getNameStart(), idLoc, type.get(), InitValue.get());
// becomes
   StmtResult Res =
       Actions.ActOnDeclaration(id->getNameStart(), idLoc, type.get(), InitValue.get());
// instead of
    StmtResult Res = Actions.ActOnDeclaration(id->getNameStart(), idLoc,
                                              type.get(), InitValue.get());


[bas]

For if() statements I always follow the Linux kernel syntax:
- allow simple if statement

Code: [Select]

if (x) func();
- allow single if() statement followed by simple else

Code: [Select]

if (x) func1();
else func2();

- if either the if or the else part needs brackets, both should have them.

so this is NOT allowed:

Code: [Select]

if (x) func1();
else {
  //...
}

Otherwise it depends on the code whether you want to use brackets or not for simple if's

[lerno]

I think any else without { } is bad, so:

Code: [Select]

if (x) func1();
else func2();

Would also be a no-go from my perspective, to the point that I would consider changing grammar so that if-else WOULD NEED to be written with blocks.

So:

Good

Code: [Select]

if (foo) bar();

if (foo) {
  bar();
} else {
  baz();
}

if (foo) {
  bar();
} else if (foo2) {
  baz();
}

Accepted by C2, but not in C2 C++ formatting

Code: [Select]

if (foo)
  bar();

Not accepted by C2, not allowed in C2's C++ formatting

Code: [Select]

if (foo) bar();
else baz();

if (foo) {
  bar();
} else baz();

if (foo) bar();
else {
  baz();
}


[bas]

We could have the analyser check that if either the if or else body has braces, that they both have. If not, just give an error.

I wouldn't really want to prevent developers from writing a short if/else and always require braces. It's a matter of style at
some point. On the todo-list is a clang-format like style program that styles C2 code. That tool could be used to check/fix
this as well. That removes it from the language and puts it in the style part...

[lerno]

I'm just in favour for making the dangling else unambiguous always. Code like

Code: [Select]

if (foo) if (bar) baz(); else foobar();


... should never ever be written. Requiring braces for if-else prevents ambiguous code.

Someone might write:

Code: [Select]

if (foo)
  if (bar) baz();
else
  foobar();


But what they get is:

Code: [Select]

if (foo)
  if (bar)
    baz();
  else
    foobar();


This is never ambiguous:

Code: [Select]

if (foo) {
  if (bar) baz();
} else {
  foobar();
}
// or
if (foo)
  if (bar) {
   baz();
  } else {
    foobar();
  }


if we allow if (foo) { ... } else ... / if (goo) ... else { ... } (that is, braces is needed for at least one part), then that would solve ambiguity as well.

We would be allowing if ( ... ) ...; so we're not locking down short ifs. Just preventing ambiguous if-else constructs.

[bas]

You can't protect against stupidity  .
If people want to write that code, let them, but don't burden good/normal developers by those restrictions..

[lerno]

Oh, it's not about stupidity, it's about grammar ambiguity.