clang-format-18 Comments alligned across scopes

[kalman5]

The following snippet

void foo() {
#if 1   // reference
#elif 0 // slower
#else   // slowest
#endif  // 0
} // foo

formattted with clang-format-17 using the following configuration

Language: Cpp

is formatted in the following way using clang-format-18

void foo() {
#if 1   // reference
#elif 0 // slower
#else   // slowest
#endif  // 0
}       // foo

as you can see the comment at the end foo is alligned with other comments inside another scope

[owenca]

Bisected to 77a38f4. @HazardyKnusperkeks can you have a look?

[HazardyKnusperkeks]

But is this wrong?
The documentation states TCAS_Always (in configuration: Always) Align trailing comments. There is no limitation of a scope.
@kalman5 why do you think it shouldn't be aligned? (Apart from that we didn't to that in the past.)

[kalman5]

Usually you want to align comments because they are related to the same context (a class, a function, a struct), alligning those across context seems wrong. Also if instead of a function you do the same with a namespace then the alignment doesn't happen. So one of the two is wrong. Gaetano

…

On Wed, Oct 4, 2023, 21:17 Björn Schäpers ***@***.***> wrote: But is this wrong? The documentation states TCAS_Always (in configuration: Always) Align trailing comments. There is no limitation of a scope. @kalman5 <https://github.com/kalman5> why do you think it shouldn't be aligned? (Apart from that we didn't to that in the past.) — Reply to this email directly, view it on GitHub <#67906 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAPIUZN3T34L3T5R5RBFMUDX5WY23AVCNFSM6AAAAAA5OEASRKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTONBXGQ4TENZVGY> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[HazardyKnusperkeks]

Also if instead of a function you do the same with a namespace then the alignment doesn't happen.

Do you refer that comments on closing namespace braces are not aligned? Yes that's the case now. Before that commit all comments on braces which are the first character on a line would not be aligned, but on all other closing braces they would be, even namespace braces. There was no concept of scope involved.
What does clang-format 17 do with:

class C {
  void foo() {
#if 1   // reference
#elif 0 // slower
#else   // slowest
#endif  // 0
  } // foo
}; // bar

Of course we could expand the non alignment to all closing braces.

And we should, regardless adapt the documentation of AlignTrailingComments.

[kalman5]

Exactly! I prefer the clang-17 behavior on this.

…

On Thu, Oct 5, 2023, 21:29 Björn Schäpers ***@***.***> wrote: Also if instead of a function you do the same with a namespace then the alignment doesn't happen. Do you refer that comments on closing namespace braces are not aligned? Yes that's the case now. Before that commit all comments on braces which are the first character on a line would not be aligned, but on all other closing braces they would be, even namespace braces. There was no concept of scope involved. What does clang-format 17 do with: class C { void foo() { #if 1 // reference #elif 0 // slower #else // slowest#endif // 0 } // foo }; // bar Of course we could expand the non alignment to all closing braces. And we should, regardless adapt the documentation of AlignTrailingComments . — Reply to this email directly, view it on GitHub <#67906 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAPIUZJZ3WDCW3R2RMQ7E4DX54DA3AVCNFSM6AAAAAA5OEASRKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTONBZGUYTSNZXGU> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[HazardyKnusperkeks]

Exactly! I prefer the clang-17 behavior on this.

That was a question, I thought you ran it through. Based on your answer you may be surprised on the actual formatting.
But here it is:

$ echo -e "class C{void foo(){\n#if 1//ref\n#elif 0//sl\n#else//slow\n#endif//0\n}//foo\n};//bar" | clang-format
class C {
  void foo() {
#if 1   // ref
#elif 0 // sl
#else   // slow
#endif  // 0
  }     // foo
};      // bar

So I open the question to @rymiel @owenca and @mydeveloperday:

1. Was my change that now all r_brace except the TT_NamespaceRBrace comments are aligned a bug fix?
2. Was it a regression and we should don't align the column 0 r_brace comments again?
3. Or extend that to all r_brace comments, regardless of their position in a line?
4. Or add option(s) to control that behavior (that I indented to do anyhow sometime, because I also don't want the comments on if, while, etc. to be aligned)?

I'm in favor of 4 or 3.

[owenca]

It seems that clang-format version 18 is more consistent than 17:

$ cat test.cpp
if (a) { // if a
  a; // a
  if (b) { // if b
    b; // b;
    foo; // foo
  } // inner
  bar; // bar
} // outer
$ clang-format-17 test.cpp
if (a) {   // if a
  a;       // a
  if (b) { // if b
    b;     // b;
    foo;   // foo
  }        // inner
  bar;     // bar
} // outer
$ clang-format-18 test.cpp
if (a) {   // if a
  a;       // a
  if (b) { // if b
    b;     // b;
    foo;   // foo
  }        // inner
  bar;     // bar
}          // outer

However, IMO it doesn't make sense to align comments across scopes. Instead, the expected output should be:

if (a) { // if a
  a; // a
  if (b) { // if b
    b;   // b;
    foo; // foo
  } // inner
  bar; // bar
} // outer

[owenca]

What does clang-format 17 do with:

class C {
  void foo() {
#if 1   // reference
#elif 0 // slower
#else   // slowest
#endif  // 0
  } // foo
}; // bar

Both versions 17 and 18 format it as:

class C {
  void foo() {
#if 1   // reference
#elif 0 // slower
#else   // slowest
#endif  // 0
  }     // foo
};      // bar

Again, IMO the closing brace comments should not be aligned.

[HazardyKnusperkeks]

A tentative fix is in the pull request, a few points are still open for debate.

[owenca]

So I open the question to @rymiel @owenca and @mydeveloperday:

1. Was my change that now all r_brace except the TT_NamespaceRBrace comments are aligned a bug fix?
2. Was it a regression and we should don't align the column 0 r_brace comments again?
3. Or extend that to all r_brace comments, regardless of their position in a line?
4. Or add option(s) to control that behavior (that I indented to do anyhow sometime, because I also don't want the comments on if, while, etc. to be aligned)?

I think something as simple as "not aligning trailing comments preceded by a wrapped group of 1 or more closing braces optionally followed by a semicolon" would suffice. For example:

namespace foo { namespace bar {
  int i;  //
  bool b; //
}} //

void f() {
  auto l = [] {
    char c;     //
    unsigned u; //
  }; //
  int i;  //
  bool b; //
} //

[owenca]

I think something as simple as "not aligning trailing comments preceded by a wrapped group of 1 or more closing braces optionally followed by a semicolon" would suffice.

To clarify, by "closing braces" I meant any types of braces including braces of try/catch, requires, and list initialization.

[HazardyKnusperkeks]

I think something as simple as "not aligning trailing comments preceded by a wrapped group of 1 or more closing braces optionally followed by a semicolon" would suffice.

To clarify, by "closing braces" I meant any types of braces including braces of try/catch, requires, and list initialization.

That should be in the newest version. (There are no tests for try, etc. I'll add them.)