Coding rules and cpplint

[peterschrammel]

No description provided.

[tautschnig]

I'm not quite sure why this one is tagged "help", or actually whether there are specific questions that need answers?

[tautschnig]

Should that be 3rd-party libraries (instead of "3rd libraries")?

[tautschnig]

Redundant with "Do not use namespaces"

[tautschnig]

s/SLT/STL/

[tautschnig]

What would be prohibited?

[tautschnig]

Suggestions for further rules:

• Each function should have a standard comment block above it (all of which should eventually get populated)
• Headers need to have include guards (`#ifndef CPROVER_.... #define CPROVER_... #endif)

[thk123]

These are the questions I've had so far on which style is prefered. I don't know if these are too detailed/heavy handed.

A ruling on spacing, e.g. prefer

int x=0;
// or
int x = 0;

Preferred header guard style, e.g.:

#ifndef FILENAME_H
#define FILENAME_H
// ...
#endif // FILENAME_H

Specify whether the override keyword should be used on function declarations.

Should {, } be always used for if, while etc. or not

if(condition)
{
  doSomething();
}
// or:
if(condition)
  doSomething();

[tautschnig]

Very useful questions, and I'm answering them from having read a lot of code (no right or wrong here, just ruling from majority of cases):

1. No whitespace around assignment operators
2. The header-guard style is unfortunately a bit mixed. Personally I would prefer some rule that is easily machine-checkable, such as the full path or containing-directory+file name.
3. I haven't seen any use of override
4. Use {...} where necessary or where it makes sense for other reasons, but don't unnecessarily stretch the code vertically.

[tautschnig]

The code base, by and large, actually uses 70 chars. Though be sensible about it: don't trade extra-short (and then meaningless) variable names for achieving short lines.

[thk123]

A couple of other things that have come up from @peterschrammel reviewing my code (apologies Peter if I've misunderstood anything):

When splitting function parameters on to multiple lines (both in headers and in source files), subsequent lines should not be indented to continue from previous line i.e.

No Continuation

void foo(int x, 
  int y, 
  int z);

instead of Continuation:

void foo(int x, 
         int y, 
         int z);

Does anyone know of a way to get QtCreator to do this? As far as I can tell it can either have no indentation or align with the previous line

Full variable names (e.g. symbol_table not st).

Prefer ++i to i++

Use American spellings

Use #if 0 to comment out code

[peterschrammel]

We'll make this machine checkable so that we don't have to comment on most style issues during reviewing anymore.

[peterschrammel]

The integration with travis is working now. The CPP-LINT log is at the end of the log.

[thk123]

Is this really what the coding rules suggest?

Old code:

mock_generator.reset(new c_fff_mock_generatort(*message_handler, *e2c,
  symbol_table));

New code:

mock_generator.reset(
  new c_fff_mock_generatort(
  *message_handler,
  *e2c,
  symbol_table));

This code is already accepted by the linter, would this be preferable?

mock_generator.reset(
  new c_fff_mock_generatort(
    *message_handler,
    *e2c,
    symbol_table));

[tautschnig]

On Tue, Nov 29, 2016 at 3:21:44 -0800, thk123 wrote: Is this really what the coding rules suggest? Old code: mock_generator.reset(new c_fff_mock_generatort(*message_handler, *e2c, symbol_table));

I suppose the linter says the above is undesirable? I would agree, because working out matching parentheses is a bit hard (i.e., is symbol_table an argument to reset or to c_fff_mock_generatort?).

New code: mock_generator.reset( new c_fff_mock_generatort( *message_handler, *e2c, symbol_table)); This code is already accepted by the linter, would this be preferable? mock_generator.reset( new c_fff_mock_generatort( *message_handler, *e2c, symbol_table));

Ideally the linter would always require that last suggestion, but I guess that's hard to do. Overall there are many aspects here that are a matter of personal taste and perception. The codebase, at present, vastly conforms with the last example given above. Best, Michael

[tautschnig]

@peterschrammel If you could squash my additional commits into your Travis configuration modifications I'd be grateful!

[peterschrammel]

@tautschnig, squashed.

[peterschrammel]

@thk123, as @tautschnig pointed out, the latter version is preferable:

mock_generator.reset(
  new c_fff_mock_generatort(
    *message_handler,
    *e2c,
    symbol_table));

[peterschrammel]

@kroening, can we merge this?