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

Alignment of consecutive assignment and declarations causes incorrect indentation for array variables #52914

Closed
JohnC32 opened this issue Dec 29, 2021 · 7 comments
Assignees
Labels
bug Indicates an unexpected problem or unintended behavior clang-format regression

Comments

@JohnC32
Copy link

JohnC32 commented Dec 29, 2021

Given

void foo() {
    const char* formats[]  = {"%",
                              "%%",  // some comment
                              "%+-?4.5d"
                              "%#-4.3%f",
                              "%15.a6e"};
    const int   numFormats = sizeof(formats) / sizeof(char*);
}

clang-format 13 will produce:

void foo() {
    const char* formats[]  = {"%",
                             "%%",  // some comment
                             "%+-?4.5d"
                              "%#-4.3%f",
                             "%15.a6e"};
    const int   numFormats = sizeof(formats) / sizeof(char*);
}

Note, this differs from clang-format 12.0.1 which is slightly better but still incorrect.

Here's the _clang-format used in the above:

---
BasedOnStyle: Google
ColumnLimit: 100
IndentWidth: 4
AlignConsecutiveAssignments: Consecutive
AlignConsecutiveDeclarations: Consecutive
...

If you comment out the AlignConsecutive* lines in _clang-format, we get correct indentation, thus the alignment items are causing problems.

---
BasedOnStyle: Google
ColumnLimit: 100
IndentWidth: 4
AlignConsecutiveAssignments: Consecutive
AlignConsecutiveDeclarations: Consecutive
...
@JohnC32 JohnC32 changed the title Alignment of consecutive assignment and declarations causes incorrect indentation for structs Alignment of consecutive assignment and declarations causes incorrect indentation for array variables Dec 29, 2021
@mkurdej mkurdej added bug Indicates an unexpected problem or unintended behavior clang-format regression and removed new issue labels Jan 1, 2022
@mkurdej
Copy link
Member

mkurdej commented Jan 1, 2022

You say that version 12 gives incorrect formatting as well. What's the expected output for you?

@JohnC32
Copy link
Author

JohnC32 commented Jan 1, 2022

With 12.0.1, I get:

void foo() {
    const char* formats[]  = {"%",
                             "%%",  // some comment
                             "%+-?4.5d"
                             "%#-4.3%f",
                             "%15.a6e"};
    const int   numFormats = sizeof(formats) / sizeof(char*);
}

Notice the 2nd through 5th items in the formats array are shifted left by one. If we remove the Align* items from the _clang-format file, we get the expected alignment with both 12.0.1 and 13.0.0. Thus, the Align* items in the _clang-format are causing one space back-shifting in 12.0.1 and one or two space back-shifting for 13.0.0 for the 2nd though 5th items in the formats array.

Here's the expected result (and what we get if we remove the Align* items from _clang-format):

void foo() {
    const char* formats[] = {"%",
                             "%%",  // some comment
                             "%+-?4.5d"
                             "%#-4.3%f",
                             "%15.a6e"};
    const int numFormats = sizeof(formats) / sizeof(char*);
}

@mkurdej
Copy link
Member

mkurdej commented Jan 3, 2022

Minimal reproduce:

char *a[]  = {"a", // comment
             "c"};
int   bbbb = 0;

with .clang-format:

BasedOnStyle: LLVM
AlignConsecutiveAssignments: Consecutive
AlignConsecutiveDeclarations: Consecutive

The bug doesn't happen when only one of Align* options is used.
It happens (2nd and following strings literals are shifted to the left), when the 2nd identifier (bbbb) is longer than the first one (including brackets, a[] here).

@mkurdej mkurdej self-assigned this Jan 3, 2022
@mkurdej mkurdej added the awaiting-review Has pending Phabricator review label Jan 3, 2022
@mkurdej
Copy link
Member

mkurdej commented Jan 3, 2022

Review: https://reviews.llvm.org/D116527.

@mydeveloperday
Copy link
Contributor

is the missing comma after the "%+-?4.5d" deliberate?

@mkurdej
Copy link
Member

mkurdej commented Jan 4, 2022

is the missing comma after the "%+-?4.5d" deliberate?

I have noticed it as well, but it doesn't change anything to the fact that the quotes are not aligned to the quote on the first line.

@mkurdej mkurdej closed this as completed in 5109737 Jan 5, 2022
@JohnC32
Copy link
Author

JohnC32 commented Jan 11, 2022

The missing comma was an accident, but the quotes should still align

@asl asl removed the awaiting-review Has pending Phabricator review label Jan 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Indicates an unexpected problem or unintended behavior clang-format regression
Projects
None yet
Development

No branches or pull requests

4 participants