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

[clang-format] Handle generic selections inside parentheses #79785

Closed
wants to merge 0 commits into from

Conversation

sstwcw
Copy link
Contributor

@sstwcw sstwcw commented Jan 29, 2024

new

while (_Generic(x, //
           long: x)(x) > x) {
}
while (_Generic(x, //
           long: x)(x)) {
}

old

while (_Generic(x, //
       long: x)(x) > x) {
}
while (_Generic(x, //
    long: x)(x)) {
}

In the first case above, the second line previously aligned to the open parenthesis. The 4 spaces did not get added by the fallback line near the end of getNewLineColumn because there was already some indentaton. Now the spaces get added explicitly.

In the second case above, without the fake parentheses, the second line did not respect the outer parentheses. Because the LastSpace field did not get set without the fake parentheses. Now the indentation of the outer level is used instead.

@llvmbot
Copy link
Collaborator

llvmbot commented Jan 29, 2024

@llvm/pr-subscribers-clang-format

Author: None (sstwcw)

Changes

new

while (_Generic(x, //
           long: x)(x) > x) {
}
while (_Generic(x, //
           long: x)(x)) {
}

old

while (_Generic(x, //
       long: x)(x) > x) {
}
while (_Generic(x, //
    long: x)(x)) {
}

In the first case above, the second line previously aligned to the open parenthesis. The 4 spaces did not get added by the fallback line near the end of getNewLineColumn because there was already some indentaton. Now the spaces get added explicitly.

In the second case above, without the fake parentheses, the second line did not respect the outer parentheses. Because the LastSpace field did not get set without the fake parentheses. Now the indentation of the outer level is used instead.


Full diff: https://github.com/llvm/llvm-project/pull/79785.diff

2 Files Affected:

  • (modified) clang/lib/Format/ContinuationIndenter.cpp (+5-2)
  • (modified) clang/unittests/Format/FormatTest.cpp (+9)
diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp
index a3eb9138b21833..a1c5a297c67c13 100644
--- a/clang/lib/Format/ContinuationIndenter.cpp
+++ b/clang/lib/Format/ContinuationIndenter.cpp
@@ -1694,8 +1694,11 @@ void ContinuationIndenter::moveStatePastFakeLParens(LineState &State,
     // Special case for generic selection expressions, its comma-separated
     // expressions are not aligned to the opening paren like regular calls, but
     // rather continuation-indented relative to the _Generic keyword.
-    if (Previous && Previous->endsSequence(tok::l_paren, tok::kw__Generic))
-      NewParenState.Indent = CurrentState.LastSpace;
+    if (Previous && Previous->endsSequence(tok::l_paren, tok::kw__Generic) &&
+        State.Stack.size() >= 2) {
+      NewParenState.Indent =
+          State.Stack.end()[-2].Indent + Style.ContinuationIndentWidth;
+    }
 
     if ((shouldUnindentNextOperator(Current) ||
          (Previous &&
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index e5e763edf5b5bf..c066107bebdd73 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -24111,6 +24111,15 @@ TEST_F(FormatTest, C11Generic) {
                "    double _Complex: dc,\n"
                "    long double _Complex: ldc)");
 
+  verifyFormat("while (_Generic(x, //\n"
+               "           long: x)(x) > x) {\n"
+               "}");
+  verifyFormat("while (_Generic(x, //\n"
+               "           long: x)(x)) {\n"
+               "}");
+  verifyFormat("x(_Generic(x, //\n"
+               "      long: x)(x));");
+
   FormatStyle Style = getLLVMStyle();
   Style.ColumnLimit = 40;
   verifyFormat("#define LIMIT_MAX(T)                   \\\n"

if (Previous && Previous->endsSequence(tok::l_paren, tok::kw__Generic) &&
State.Stack.size() >= 2) {
NewParenState.Indent =
State.Stack.end()[-2].Indent + Style.ContinuationIndentWidth;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is freaky, never thought I'd ever see operator[] on iterators used, and in such a way.

Comment on lines 1698 to 1700
State.Stack.size() >= 2) {
NewParenState.Indent =
State.Stack.end()[-2].Indent + Style.ContinuationIndentWidth;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be consistent with the other dozen similar sinppets:

Suggested change
State.Stack.size() >= 2) {
NewParenState.Indent =
State.Stack.end()[-2].Indent + Style.ContinuationIndentWidth;
State.Stack.size() > 1) {
NewParenState.Indent =
State.Stack[State.Stack.size() - 2].Indent + Style.ContinuationIndentWidth;

sstwcw added a commit that referenced this pull request Feb 6, 2024
new

```C
while (_Generic(x, //
           long: x)(x) > x) {
}
while (_Generic(x, //
           long: x)(x)) {
}
```

old

```C
while (_Generic(x, //
       long: x)(x) > x) {
}
while (_Generic(x, //
    long: x)(x)) {
}
```

In the first case above, the second line previously aligned to the open
parenthesis.  The 4 spaces did not get added by the fallback line near
the end of getNewLineColumn because there was already some indentaton.
Now the spaces get added explicitly.

In the second case above, without the fake parentheses, the second line
did not respect the outer parentheses, because the LastSpace field did
not get set without the fake parentheses.  Now the indentation of the
outer level is used instead.
@sstwcw sstwcw closed this Feb 6, 2024
@sstwcw sstwcw deleted the format-generic branch February 6, 2024 05:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants