Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

treat all labeling constructs consistently by allowing for an offset in the _clang-format options file #53717

Open
JohnC32 opened this issue Feb 10, 2022 · 2 comments
Labels
clang-format enhancement Improving things as opposed to bug fixing, e.g. new or missing feature

Comments

@JohnC32
Copy link

JohnC32 commented Feb 10, 2022

Please provide the ability to specify offsets for all code labeling constructs that mark (tag) code. C++ has three forms of code labeling constructs:

  1. class/struct access specifiers

    The public:, package:, and private: specifiers are "labeling constructs" that mark the code that follows as having public, package, or private access.

  2. case labels

    Case labels mark code as the target of switch conditions.

  3. Named labels

    Named labels mark code as the targets for goto statements.

clang-format lets us specify an offset for access specifiers but not the other two. For example, given this _clang-format

---
BasedOnStyle: Google
IndentWidth: 4
ColumnLimit: 100
AccessModifierOffset: -2
IndentCaseLabels: false
IndentGotoLabels: true
...

We get

class Foo {
  public:
    int f(int a) {
        int b = -1;

        if (a > 100) {
            goto LABEL;
        }

        if (a == 1) {
            b = 1;
        } else {
            b = 0;
        }

        switch (b) {
        case 1:
            b = 1;
            break;
        default:
            b = 0;
            break;
        }


    LABEL:
        return b;
    }

  private:
    void g();
};

We would like a way to treat all labeling constructs consistently and offset them the same way. Specifically, we'd like to have:

// Desired behavior: all labels offset by 2
class Foo {
  public:                // access specifier offset by 2 (currently supported by clang-format)
    int f(int a) {
        int b = -1;

        if (a > 100) {
            goto LABEL;
        }

        if (a == 1) {
            b = 1;
        } else {
            b = 0;
        }

        switch (b) {
          case 1:        // Want case labels offset by 2
            b = 1;
            break;
          default:       // Want case labels offset by 2
            b = 0;
            break;
        }


      LABEL:            // Want labels offset by 2
        return b;
    }

  private:
    void g();
};

The complexity of code is directly related to the indent level. We did an informal study on what indent level to use. We choose four spaces. Using four spaces encourages writing less complex code. Overly complex code (with too many levels of nesting) is more apparent when using four spaces to indent than when two spaces. Code that is shifted greatly to the right often indicates there is too much nesting, which is indicative of a complexity problem. Various coding standards highlight this point, such as the Linux kernel advocating for eight spaces. Using eight spaces makes it difficult to write overly complex code because the code is shifted so far to the right. In our informal study we found that eight spaces did not offer significant advantage over four spaces, however using eight spaces made it very difficult to write C++ code that looked good with a column limit of 100 because C++ has library names and type specifications that can be lengthy.

clang-format does have the ability to indent case labels, but that creates an inconsistency in complexity. For example, if you see code indented by eight spaces and are using an indent with of four, you can assume there are two conditions causing the indent of eight spaces. Specifically, these two pieces of code have the same complexity, but the indent indicates otherwise:

        if (a == 1) {
            b = 1;
        } else {
            b = 0;
        }

        switch (b) {
            case 1:
                b = 1;
                break;
            default:
                b = 0;
                break;
        }

Please add offsets for case labels and goto labels. Proposal: add CaseLabelsOffset and GotoLabelsOffset which would give the above desired behavior. For example:

---
BasedOnStyle: Google
IndentWidth: 4
ColumnLimit: 100
AccessModifierOffset: -2
IndentCaseLabels: false
CaseLabelsOffset: 2
IndentGotoLabels: true
GotoLabelsOffset: 2
...

Thanks for the excellent clang-format tool!

@EugeneZelenko EugeneZelenko added clang-format enhancement Improving things as opposed to bug fixing, e.g. new or missing feature and removed new issue labels Feb 10, 2022
@llvmbot
Copy link
Collaborator

llvmbot commented Feb 10, 2022

@llvm/issue-subscribers-clang-format

@JohnC32
Copy link
Author

JohnC32 commented Sep 15, 2022

Are there any items we could do to help with this enhancement?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang-format enhancement Improving things as opposed to bug fixing, e.g. new or missing feature
Projects
None yet
Development

No branches or pull requests

3 participants