Modify

Opened 3 years ago

Last modified 2 years ago

#1842 new defect

clang-format options not valid for versions <=3.5

Reported by: Frank Löffler Owned by:
Priority: unset Milestone:
Component: Cactus Version: development version
Keywords: Cc:

Description

The current .clang-format file is not valid for clang-format versions 3.5 and below. The attached patch would 'fix' this by removing key words not known in version 3.5, but still wouldn't work for versions below that. I do not suggest to apply the patch - it should just show that this is quite some number.

Looking at the .clang-format file it seems that it does not use one of the pre-defined styles, but rather creates it's own (even if it is based on one given the comment in the file). I wonder if all of this is really necessary, or if we could not simply use one of the pre-defined styles, possibly with a few changes that work across clang-format versions.

Attachments (3)

clang-format (1.6 KB) - added by Frank Löffler 3 years ago.
cf3.4 (1.4 KB) - added by Frank Löffler 3 years ago.
default llvm style options for clang-format-3.4
cf3.5 (1.7 KB) - added by Frank Löffler 3 years ago.
default llvm style options for clang-format-3.5

Download all attachments as: .zip

Change History (12)

Changed 3 years ago by Frank Löffler

Attachment: clang-format added

comment:1 Changed 3 years ago by Erik Schnetter

The .clang-format file is exactly the (default) LLVM style, with the exception of setting "Standard" to "Cpp03".

Changed 3 years ago by Frank Löffler

Attachment: cf3.4 added

default llvm style options for clang-format-3.4

Changed 3 years ago by Frank Löffler

Attachment: cf3.5 added

default llvm style options for clang-format-3.5

comment:2 in reply to:  1 Changed 3 years ago by Frank Löffler

Replying to eschnett:

The .clang-format file is exactly the (default) LLVM style, with the exception of setting "Standard" to "Cpp03".

That may be true for version 3.6. These don't work for earlier versions. The output of

clang-format-XXX -style=llvm -dump-config

is attached to the ticket - and they are different from the current state in the flesh, and are different from each other.

comment:3 Changed 3 years ago by Frank Löffler

I tried the following:

---
BasedOnStyle:  LLVM
Standard:      Cpp03
...

If I read the documentation correctly, this should result in a style based on LLVM, but with the standard being changed - which I also understood you wanted?

While a quick check using this and version 3.5 works, it reveals still differences to what is currently in Carpet:

$ git diff | wc -l
178

The same using version 3.4 also works, but shows more changes:

$ git diff | wc -l
687

The same happens without .clang-format for

clang-format-XXX -style="{Standard: Cpp03}" -i *.cc *.hh

I don't have version 3.6 or newer.

This to me indicates that different versions of clang-format format 'llvm-style' differently. But we also cannot (easily) specify our own style, since the options necessary to do so don't seem to work across versions.

Unless I misunderstood something (perfectly possible), we either have to dictate a certain version of clang-format (difficult to enforce and might still break existing editor-build-setups), or we would have to come up with a configuration that works at least across some commonly installed versions of clang-format in a consistent manner.

comment:4 Changed 3 years ago by Frank Löffler

I also tried using the config from version 3.4 with version 3.5 - to no avail (Standard it set to Cpp03 there by default):

$ clang-format-3.4 -style=llvm -dump-config > .clang-format
$ clang-format-3.4 -style="{Standard: Cpp03}" -i *.cc *.hh
$ git diff | wc -l
687$ clang-format-3.4 -style=file -i *.cc *.hh
$ git diff | wc -l
687
$ clang-format-3.5 -style="{Standard: Cpp03}" -i *.cc *.hh
$ git diff | wc -l
178
$ clang-format-3.5 -style=file -i *.cc *.hh
$ git diff | wc -l
178

comment:5 Changed 3 years ago by Erik Schnetter

Since the global .clang-format file doesn't work, the option to use seems to be:

clang-format -style="{BasedOnStyle: LLVM, Standard: Cpp03}"

Unfortunately, it seems that different clang-format versions make slightly different decisions regarding how to break lines or how to interpret ampersand characters. (The latter are difficult to interpret if one doesn't have full type information.) It thus seems that re-formatting everything with different versions of clang-format leads to changes.

I'm using clang-format 3.7, the latest released version. Clang is relatively easy to install (easier than GCC), and we should be able to decide on a single version for the Einstein Toolkit. For example, Ubuntu LTS currently provides version 3.6.

comment:6 Changed 3 years ago by Roland Haas

Which version was used for the already formated thorns? We should use that one otherwise we have to re-format everything if the chosen version produces a different formatting.

comment:7 in reply to:  5 Changed 3 years ago by Frank Löffler

Replying to eschnett:

I'm using clang-format 3.7, the latest released version. Clang is relatively easy to install (easier than GCC), and we should be able to decide on a single version for the Einstein Toolkit. For example, Ubuntu LTS currently provides version 3.6.

Debian (stable) has version 3.5 (and 3.4), not 3.6 or newer.

comment:8 Changed 2 years ago by Ian Hinder

3.9, the latest version available in MacPorts, also produces changes on current master. But do we know that current master is "clean" with respect to 3.7, which Erik was using 5 months ago (comment:5)?

comment:9 Changed 2 years ago by Ian Hinder

Installing using MacPorts pulls in many gigabytes of files, including all of clang and all of llvm. It is much easier to install using homebrew;

$ brew install clang-format
==> Downloading https://homebrew.bintray.com/bottles/clang-format-2015-04-21.mavericks.bottle.tar.gz
######################################################################## 100.0%
==> Pouring clang-format-2015-04-21.mavericks.bottle.tar.gz
   /usr/local/Cellar/clang-format/2015-04-21: 5 files, 1.4M

This gives

clang-format version 3.7.0 (tags/google/testing/2015-04-21)

which is the version Erik mentioned using. However, this version also produces changes in current master, similar to the ones I got in 3.9. I suspect that current master is not "clean". Erik, please can you check?

Modify Ticket

Change Properties
Set your email in Preferences
Action
as new The ticket will remain with no owner.
Next status will be 'review'.
as The resolution will be set.
to The owner will be changed from (none) to the specified user.
Next status will be 'confirmed'.
The owner will be changed from (none) to anonymous.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.