clang-format options not valid for versions <=3.5

Create issue
Issue #1842 new
Frank Löffler created an issue

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.

Keyword:

Comments (9)

  1. Erik Schnetter
    • removed comment

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

  2. Frank Löffler reporter
    • removed comment

    Replying to [comment:1 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.

  3. Frank Löffler reporter
    • removed comment

    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.

  4. Frank Löffler reporter
    • removed comment

    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
    
  5. Erik Schnetter
    • removed comment

    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.

  6. Roland Haas
    • removed comment

    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.

  7. Frank Löffler reporter
    • removed comment

    Replying to [comment:5 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.

  8. Ian Hinder
    • removed comment

    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)?

  9. Ian Hinder
    • removed comment

    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?

  10. Log in to comment