search

Quuxplusone / LLVMBugzillaTest

0 stars 0 forks source link

clang-format vertical alignment of 2d array declarations, arrays of structs, and excessively long arrays. #39872

Open Quuxplusone opened 5 years ago

Quuxplusone commented 5 years ago
Bugzilla Link PR40906
Status NEW
Importance P enhancement
Reported by Micah Snyder (micasnyd@cisco.com)
Reported on 2019-02-28 12:35:32 -0800
Last modified on 2020-03-20 17:17:48 -0700
Version unspecified
Hardware PC All
CC djasper@google.com, klimek@google.com, llvm-bugs@lists.llvm.org, pmenzel+bugs.llvm.org@molgen.mpg.de, vlovich@gmail.com
Fixed by commit(s)
Attachments
Blocks
Blocked by
See also 
Two-dimensional arrays, arrays of structs, and excessively long single-
dimension arrays that are formatted by clang-format end up being
extraordinarily lengthy as each element in each struct will be assigned a
separate line instead of being organized as a matrix.

Ideally, clang-format would format the 2nd dimension, or the entire struct (in
an array of structs), on one line, with each member variable aligned vertically
so as to appear as a table or matrix.

The following examples showing the need for this feature/improvement are from
the ClamAV project. I will note that for the time being we also disable clang-
format to align consecutive macros, which is a feature already in
[review](https://reviews.llvm.org/D28462) that I am looking forward to using.

Array of Structs Declarations:

* libclamav/filestypes.c:
  * https://github.com/Cisco-Talos/clamav-devel/blob/dev/0.102/libclamav/filetypes.c#L57
  * https://github.com/Cisco-Talos/clamav-devel/blob/dev/0.102/libclamav/filetypes.c#L226

* https://github.com/Cisco-Talos/clamav-
devel/blob/dev/0.102/libclamav/matcher.h#L183
  * Note, this one contains edge case where the 2nd dimension contains an additional struct as a member variable.

* https://github.com/Cisco-Talos/clamav-
devel/blob/dev/0.102/libclamav/msxml.c#L52

* https://github.com/Cisco-Talos/clamav-
devel/blob/dev/0.102/libclamav/swf.h#L131

2D Array Declarations:

* https://github.com/Cisco-Talos/clamav-
devel/blob/dev/0.102/libclamav/htmlnorm.c#L129

* https://github.com/Cisco-Talos/clamav-
devel/blob/dev/0.102/libclamav/pe_icons.c#L254

* libclamav/disasm.c contains many examples, although the variables in these
examples are unfortunately not aligned:
  * https://github.com/Cisco-Talos/clamav-devel/blob/dev/0.102/libclamav/disasm.c#L83
  * https://github.com/Cisco-Talos/clamav-devel/blob/dev/0.102/libclamav/disasm.c#L94
  * https://github.com/Cisco-Talos/clamav-devel/blob/dev/0.102/libclamav/disasm.c#L105
  * https://github.com/Cisco-Talos/clamav-devel/blob/dev/0.102/libclamav/disasm.c#L114
  * etc...

A related issue with single-dimension arrays where existing line-breaks need to
be respected is probably not addressable.

* https://github.com/Cisco-Talos/clamav-
devel/blob/dev/0.102/libclamav/sf_base64decode.c#L31

* https://github.com/Cisco-Talos/clamav-
devel/blob/dev/0.102/libclamav/binhex.c#L36

* https://github.com/Cisco-Talos/clamav-
devel/blob/dev/0.102/libclamav/htmlnorm.c#L103

* https://github.com/Cisco-Talos/clamav-devel/blob/dev/0.102/libclamav/matcher-
ac.c#L71

* https://github.com/Cisco-Talos/clamav-
devel/blob/dev/0.102/libclamav/textdet.c#L58

Additional detail about how the ClamAV project is using clang-format are listed
in this blog post:
https://blog.clamav.net/2019/02/clamav-adopts-clang-format.html
Quuxplusone commented 4 years ago

To add some additional color, the workaround is to turn off clang-format around vertically-aligned code sections. However that can result in linter issues getting through (e.g. someone inserts a tab or space in the wrong spot). Being able to annotate code as desirable for being vertically aligned (& thus violate other constraints like line length or have an alternate line length) would be fantastic for these corner cases IMHO.