[MDEV-13631] Make use of clang-format Created: 2017-08-23  Updated: 2019-10-02  Resolved: 2019-05-30

Status: Closed
Project: MariaDB Server
Component/s: Scripts & Clients
Fix Version/s: 10.0.38, 10.2.25, 5.5.65, 10.1.41, 10.3.16, 10.4.6

Type: Task Priority: Major
Reporter: Eugene Kosov (Inactive) Assignee: Sergey Vojtovich
Resolution: Fixed Votes: 1
Labels: None


 Description   

It's a proposal, not a bug.

The main reason to use clang-format: it increases productivity. It will be easy to write code and more importantly to read a well-formatted code afterwards.

clang-format is available on win and *nix.

It has a nice integration with popular editors (e.g. https://github.com/llvm-mirror/clang/blob/master/tools/clang-format/clang-format.el, http://llvm.org/builds/)

It allows to easily enforce some coding style.

It's easy to format whole code base once (producing a huge patch though) and it's possible to add a pre-commit hook to invoke clang-format.

Introductory video:
https://www.youtube.com/watch?v=s7JmdCfI__c

Official documentation:
http://clang.llvm.org/docs/ClangFormat.html
http://clang.llvm.org/docs/ClangFormatStyleOptions.html

/sql config could look like this:

BreakBeforeBraces: Allman
SpaceBeforeAssignmentOperators: false

/storage/innobase config could look like this:

UseTab: Always
TabWidth: 8
IndentWidth: 8



 Comments   
Comment by Eugene Kosov (Inactive) [ 2017-12-04 ]

A bit more precise InnoDB style:

UseTab: Always
TabWidth: 8
IndentWidth: 8
BreakBeforeBinaryOperators: NonAssignment
PointerAlignment: Left
AlwaysBreakAfterReturnType: TopLevel
BinPackParameters: true
ContinuationIndentWidth: 8
BreakBeforeBraces: Custom
BraceWrapping:
  AfterFunction: true

Some scripts like integration with IDE, git-clang-format, clang-format-diff: https://github.com/llvm-mirror/clang/tree/master/tools/clang-format

Comment by Aleksey Midenkov [ 2017-12-05 ]

The task itself is rather questionable: "Make use of clang-format". Why it should be used? The goal should be paramaunt but not the instrument. If the task is about increasing productivity, then how it helps achieving it?

As I see from the video it is just a very intelligent formatter, like crustify, but maybe better. But not more: it won't do things beyond formatting and this is rather minor part of productivity.

Using auto-formatters without human control in scope of VCS is also questionable: formatting garbles history. It may touch the lines that should not be touched by a patch.

Machine formatting even very smart is impossible to handle 100% of cases to supply good readability. While it induces lazyness, like "everything is under coding style conditions" it produces something like this:

  if (thd->work_part_info &&
      !(create_info->db_type->flags & HTON_NATIVE_SYS_VERSIONING) &&
      thd->work_part_info->default_engine_type &&
      (thd->work_part_info->default_engine_type->flags &
       HTON_NATIVE_SYS_VERSIONING))

Is it readable? I wouldn't say so. Is it productive? While I try to read this code and modify it to be more readable I spend efforts which makes it not very productive.

How it can help to be productive? I see it as a series of config files in src trees `sql/`, `storage/innodbase`, etc. Where developer may (but not must) use it as a helping aid to do local formatting (as there are different coding styles in different subtrees). This conception is not different from .editorconfig, .kateconfig, etc which are good helping aid to apply correct editor settings. See info on editorconfig.

.editorconfig

root = true
 
[*]
end_of_line = lf
insert_final_newline = true
trim_trailing_whitespace = false
 
[Makefile]
indent_style = tab

./sql/.editorconfig

[*]
indent_style = space
indent_size = 2
tab_width = 8

storage/innobase/.editorconfig

[*]
indent_style = tab
indent_size = tab
tab_width = 8
end_of_line = lf
insert_final_newline = true
trim_trailing_whitespace = false

These files should be also in source tree along with auto-formatter configs.

Comment by Sergey Vojtovich [ 2019-05-30 ]

Let's have proposed patch as a starting point. I read a bit about editorconfig, clang-format seems to be superior. Comments like "I loved EditorConfig until tools like clang-format and gofmt showed up" don't seem to be uncommon.

Comment by Aleksey Midenkov [ 2019-10-02 ]

My point is: it's not important what tool to use. There can be many configs for many tools for different goals. The editorconfig is an example of tool that applies config to your editor depending on file location. This is not a replacement for clang-format of course.

Generated at Thu Feb 08 08:07:06 UTC 2024 using Jira 8.20.16#820016-sha1:9d11dbea5f4be3d4cc21f03a88dd11d8c8687422.