Uploaded image for project: 'Request For Comments'
  1. Request For Comments
  2. RFC-261

Automate C++ code layout with clang-format

    XMLWordPrintable

    Details

    • Type: RFC
    • Status: Implemented
    • Resolution: Done
    • Component/s: DM
    • Labels:
      None

      Description

      Currently section 6 of our C++ style guide (https://developer.lsst.io/coding/cpp_style_guide.html1) has lots of rules relating to code formatting / layout. Following these rules consistently takes time. When it is not done consistently the code looks different resulting in a higher cognitive load when reading. Moreover, digesting the style guide itself also costs time.

      For C++ the clang-format (http://clang.llvm.org/docs/ClangFormat.html1) tool enables automatic code layout / formatting for C++. It has standardized profiles complying with the LLVM, Google and Mozilla coding styles and a configuration system (http://clang.llvm.org/docs/ClangFormatStyleOptions.html) to further specialize those styles. If even more control is required the libformat library can be used to build our own formatting tool.

      This RFC proposes that we add a clang-format style file to the DM C++ developer guide (version / change controlled with it) and replace all relevant points (anything that can be handled by the formatting tool) with: "code shall be formatted identically to that produced by running it through clang-format using the LSST style file".

      This way we leave open the possibility for users to use any tool they desire, or do the layout by hand as long as the result is the same.

      This proposal was discussed on community. For more details about that discussion, including examples of formatted code, see: https://community.lsst.org/t/format-c-code-using-clang-format/1464

        Attachments

          Issue Links

            Activity

            Hide
            ktl Kian-Tat Lim added a comment -

            The general principle of having our style guide be deltas on a well-known style is fine. As long as no actual change to the resulting style is proposed, unlike Russell Owen's suggestion or my own about spaces around certain binary operators, I'm OK with adopting this, again, subject to review of the actual text.

            Show
            ktl Kian-Tat Lim added a comment - The general principle of having our style guide be deltas on a well-known style is fine. As long as no actual change to the resulting style is proposed, unlike Russell Owen 's suggestion or my own about spaces around certain binary operators, I'm OK with adopting this, again, subject to review of the actual text.
            Hide
            pschella Pim Schellart [X] (Inactive) added a comment -

            Kian-Tat Lim has agreed with adopting this via slack. Final veto will be on PR of implementation.

            Show
            pschella Pim Schellart [X] (Inactive) added a comment - Kian-Tat Lim has agreed with adopting this via slack. Final veto will be on PR of implementation.
            Hide
            tjenness Tim Jenness added a comment -

            Pim Schellart [X] please add the triggered DM tickets so that we know what work is going to be done for this ticket to be implemented.

            Show
            tjenness Tim Jenness added a comment - Pim Schellart [X] please add the triggered DM tickets so that we know what work is going to be done for this ticket to be implemented.
            Hide
            pschella Pim Schellart [X] (Inactive) added a comment -

            The triggered implementation issue is DM-8713.

            The intention of this RFC is indeed to make the DM C++ Style Guide rules concerning code layout (section 6) into a delta on clang-format (with the delta covering items not handled by the tool).
            However, this explicitly does require changing some of the rules. Because, even with options tweaked to match current conventions as closely as possible, it is not possible to reproduce our current rules exactly.
            Unlike autopep8, because clang-format operates on the AST generated by clang it cannot simply ignore certain rules and keep existing formatting (unless explicitly marked). It simply doesn't remember what the old formatting looked like. For instance, it always puts spaces around all binary operators and this is not configurable (without modifying clang-format itself).

            Show
            pschella Pim Schellart [X] (Inactive) added a comment - The triggered implementation issue is DM-8713 . The intention of this RFC is indeed to make the DM C++ Style Guide rules concerning code layout (section 6) into a delta on clang-format (with the delta covering items not handled by the tool). However, this explicitly does require changing some of the rules. Because, even with options tweaked to match current conventions as closely as possible, it is not possible to reproduce our current rules exactly. Unlike autopep8 , because clang-format operates on the AST generated by clang it cannot simply ignore certain rules and keep existing formatting (unless explicitly marked). It simply doesn't remember what the old formatting looked like. For instance, it always puts spaces around all binary operators and this is not configurable (without modifying clang-format itself).
            Hide
            rowen Russell Owen added a comment -

            I'll put in a vote for simple rules for binary operators (e.g. one space each side, always). Our current LSST rules do enhance readability in some cases, but not in others. I find them overly fussy and I dislike the fact that they disagree with PEP8 and other standards.

            To put it another way, it sounds as if we have to give up our special rules for C++, so we might as well also give them up for Python.

            Show
            rowen Russell Owen added a comment - I'll put in a vote for simple rules for binary operators (e.g. one space each side, always). Our current LSST rules do enhance readability in some cases, but not in others. I find them overly fussy and I dislike the fact that they disagree with PEP8 and other standards. To put it another way, it sounds as if we have to give up our special rules for C++, so we might as well also give them up for Python.

              People

              Assignee:
              pschella Pim Schellart [X] (Inactive)
              Reporter:
              pschella Pim Schellart [X] (Inactive)
              Watchers:
              John Parejko, John Swinbank, Kian-Tat Lim, Krzysztof Findeisen, Pim Schellart [X] (Inactive), Russell Owen, Tim Jenness
              Votes:
              2 Vote for this issue
              Watchers:
              7 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:
                Planned End:

                  Jenkins

                  No builds found.