Re: [omr-dev] our coding standard

[Matthew Gaudet <magaudet@xxxxxxxxxxxxxxxxxx>]

On 9/1/2016 10:36 PM, Daniel Heidinga wrote:
> I went through the branches Matthew had posted (thanks for doing that!)

No problem :D

> and compared the formatted code to the original using
> omr/hashtable/hashtable.c.

One thing I'd encourage everyone to do is also look at the C++ code
handling. An appreciable fraction of the changes that will be made
because of C++ specific style tweaks.

> I agree that 'webkit' is closest to our
> current standard (though I strongly prefer to use tabs rather than
> spaces as it helps avoid the ascii-art alignment in code).  For most of
> the standards, the lines are very short when formatted.  Can this be
> tweaked?

Everything can be tweaked with enough effort :D Below my signature I've
dumped the precise definition of what clang-format-3.8 considers to be
'webkit' style. The relevant parameter would be ColumnLimit, which is
set to zero (disabling a column limit). Other styles could have that
added.

The clang-format configurator website [1] is a very visual way of
exploring format options.


> It also doesn't address the issue of missing braces for control flow (if
> / while / etc).  A separate tool (clang-tidy?) would be needed to
> address these kinds of issues.  If we're requiring one, do we want to
> require both tools?

I'd absolutely support us getting to the point where we could use
clang-tidy, and write our own checks and fixes. The clang-tidy tool
you're looking for is readability-braces-around-statements[2].



> My overall impression is that the autoformatter doesn't actually get
> us to consistent code, it just introduces auto-generated
> inconsistencies.

Certainly the transition introduces inconsistencies. However, the tool
once formatted and deployed, should then be relatively stable: Changes
the tools makes subsequently should be driven by code changes.

In my opinion having the tool make these decisions for me is incredibly
valuable, saving me huge amounts of effort.

For me: I will run clang-format religiously, whatever style someone
gives me. I prefer something like webkit or llvm, for the simple reason
that they're built in. Personally, if someone else is willing to develop
and maintain a style file for the OMR style, I'll use it. I just don't
want to be the one who maintains it.

-- Matthew

[1]: http://zed0.co.uk/clang-format-configurator/
[2]: 
http://clang.llvm.org/extra/clang-tidy/checks/readability-braces-around-statements.html

---
Language:        Cpp
# BasedOnStyle:  WebKit
AccessModifierOffset: -4
AlignAfterOpenBracket: DontAlign
AlignConsecutiveAssignments: false
AlignConsecutiveDeclarations: false
AlignEscapedNewlinesLeft: false
AlignOperands:   false
AlignTrailingComments: false
AllowAllParametersOfDeclarationOnNextLine: true
AllowShortBlocksOnASingleLine: false
AllowShortCaseLabelsOnASingleLine: false
AllowShortFunctionsOnASingleLine: All
AllowShortIfStatementsOnASingleLine: false
AllowShortLoopsOnASingleLine: false
AlwaysBreakAfterDefinitionReturnType: None
AlwaysBreakAfterReturnType: None
AlwaysBreakBeforeMultilineStrings: false
AlwaysBreakTemplateDeclarations: false
BinPackArguments: true
BinPackParameters: true
BraceWrapping:
  AfterClass:      false
  AfterControlStatement: false
  AfterEnum:       false
  AfterFunction:   true
  AfterNamespace:  false
  AfterObjCDeclaration: false
  AfterStruct:     false
  AfterUnion:      false
  BeforeCatch:     false
  BeforeElse:      false
  IndentBraces:    false
BreakBeforeBinaryOperators: All
BreakBeforeBraces: WebKit
BreakBeforeTernaryOperators: true
BreakConstructorInitializersBeforeComma: true
ColumnLimit:     0
CommentPragmas:  '^ IWYU pragma:'
ConstructorInitializerAllOnOneLineOrOnePerLine: false
ConstructorInitializerIndentWidth: 4
ContinuationIndentWidth: 4
Cpp11BracedListStyle: false
DerivePointerAlignment: false
DisableFormat:   false
ExperimentalAutoDetectBinPacking: false
ForEachMacros:   [ foreach, Q_FOREACH, BOOST_FOREACH ]
IncludeCategories:
  - Regex:           '^"(llvm|llvm-c|clang|clang-c)/'
    Priority:        2
  - Regex:           '^(<|"(gtest|isl|json)/)'
    Priority:        3
  - Regex:           '.*'
    Priority:        1
IndentCaseLabels: false
IndentWidth:     4
IndentWrappedFunctionNames: false
KeepEmptyLinesAtTheStartOfBlocks: true
MacroBlockBegin: ''
MacroBlockEnd:   ''
MaxEmptyLinesToKeep: 1
NamespaceIndentation: Inner
ObjCBlockIndentWidth: 4
ObjCSpaceAfterProperty: true
ObjCSpaceBeforeProtocolList: true
PenaltyBreakBeforeFirstCallParameter: 19
PenaltyBreakComment: 300
PenaltyBreakFirstLessLess: 120
PenaltyBreakString: 1000
PenaltyExcessCharacter: 1000000
PenaltyReturnTypeOnItsOwnLine: 60
PointerAlignment: Left
ReflowComments:  true
SortIncludes:    true
SpaceAfterCStyleCast: false
SpaceBeforeAssignmentOperators: true
SpaceBeforeParens: ControlStatements
SpaceInEmptyParentheses: false
SpacesBeforeTrailingComments: 1
SpacesInAngles:  false
SpacesInContainerLiterals: true
SpacesInCStyleCastParentheses: false
SpacesInParentheses: false
SpacesInSquareBrackets: false
Standard:        Cpp03
TabWidth:        8
UseTab:          Never
...