We found another regression with this in wrongly indenting/not putting on its own line ObjC @interface:

% ./clang-format --version           
clang-format version 15.0.0 (https://github.com/llvm/llvm-project.git 50cd52d9357224cce66a9e00c9a0417c658a5655)
% cat test.m
NS_SWIFT_NAME(A)
@interface B : C
@property(readonly) D value;
@end

It looks like this regressed the following example by adding an unwanted level of indentation to the #elif B branch:

% ./clang-format --version
clang-format version 15.0.0 (https://github.com/llvm/llvm-project.git 50cd52d9357224cce66a9e00c9a0417c658a5655)
% cat test.cc             
#define MACRO_BEGIN

In D121370#3488401, @kwk wrote:

@krasimir could you please test this patch on your side?

@kwk thank you! the new version of the patch looks good!

• update with just the last commit

@krasimir I'm on sick leave and will have a look as soon as I'm back. Can it wait that long?

It appears that this regressed include sorting in the following case, where the contents of test.h show the expected include order and the clang-format behavior before this patch:

% cat test.h
#include <cstdint>

It appears that this regressed some ObjC patterns:

So I'd rather keep the old behaviour for # in Proto but not in C-like languages. Is that acceptable?

It appears that this regressed some ObjC patterns:

% cat test.m  # before: good
void f() {
  //
  BOOL a = [b.c count] > 1;
  int b = a;
}
% clang-format -style=google test.m  # after: bad, statements joined on the same line
void f() {
  //
  BOOL a = [b.c count] > 1; int b = a;
}
%

This is getting more fun: the reason why /// isn't broken up is because the first character / after the comment leader // is considered punctuation per isPunctuation's CHAR_RAWDEL, which includes all these {}[]#<>%:;?*+-/^&|~!=,"':

• https://github.com/llvm/llvm-project/blob/73a15ad567071c13e761fd6d593e2b66164865e2/clang/lib/Format/BreakableToken.cpp#L790
• https://github.com/llvm/llvm-project/blob/73a15ad567071c13e761fd6d593e2b66164865e2/clang/include/clang/Basic/CharInfo.h#L139
• https://github.com/llvm/llvm-project/blob/73a15ad567071c13e761fd6d593e2b66164865e2/clang/include/clang/Basic/CharInfo.h#L31

In D121451#3395611, @curdeius wrote:

In D121451#3395578, @krasimir wrote:

This appears to have broken a relatively common pattern we see in text proto comments where sections use a style like this:

################################################################################
# Big section name
################################################################################

That's a pity.
Do you think that just doing a different thing (what we had before) for Proto (and if possible other languages where # is used in a similar way) should be enough?

This appears to have broken a relatively common pattern we see in text proto comments where sections use a style like this:

################################################################################
# Big section name
################################################################################

Thank you!
Turns out the original source wasn't using an ascii space for the comment indentation but a unicode [c2 a0] no-break space:

2f 2f c2 a0 43 6f 6d 6d  65 6e 74 0a 69 6e 74 20  |//..Comment.int |

IMO we shouldn't aim to handle non-ascii spaces for indentation in clang-format; the new behavior on this example is fine.

It appears that this caused a regression by adding an additional space of indentation to line comments in some cases:

% cat test.cc  # (before)
// Comment
int i;
% clang-format -style=google test.cc
//  Comment
int i;

@ksyx could you please take a look?

I just pushed 6cb42cd6669785f3b611106e1b6b38bbe65733a9 to hopefully fix this.

@achieveartificialintelligence thank you for looking into this, sorry for the late reply!

In D93298#3342480, @achieveartificialintelligence wrote:

In D93298#3342452, @krasimir wrote:

@achieveartificialintelligence any progress with the problematic IR? Have you been able to reproduce using nikic's reduced example?

Sorry, I don't have a solution right now.

@achieveartificialintelligence any progress with the problematic IR? Have you been able to reproduce using nikic's reduced example?

Thank you!

• address review comments

It appears that this causes a regression by adding a space in-between pointer dereferences *p -> * p in some cases, e.g. formatting this with llvm style:

// before
void f() {
  while (p < a && *p == 'a')
    p++;
}
// after
void f() {
  while (p < a && * p == 'a')
    p++;
}

It appears that after this patch clang-format started breaking up requires in javascript, e.g.:

// before
function f() {
  var requires = {};
}
// after
function f() {
  var
    requires
  = {};
}

It appears that this is causing an assertion segfault in a rustc test over at our experimental rust + llvm@head bot:
https://buildkite.com/llvm-project/rust-llvm-integrate-prototype/builds/8430#167e6de5-2dd5-41c3-87d7-b6e3f3908371/262-706
The test is https://github.com/rust-lang/rust/blob/master/src/test/assembly/asm/riscv-types.rs. These two lines appear to cause it (code compiles fine when removed):

• check_reg!(a0_f32 f32 "a0" "mv"); https://github.com/rust-lang/rust/blob/f838a425e3134d036a7d9632935111a569ac7446/src/test/assembly/asm/riscv-types.rs#L178
• check_reg!(a0_f64 f64 "a0" "mv"); https://github.com/rust-lang/rust/blob/f838a425e3134d036a7d9632935111a569ac7446/src/test/assembly/asm/riscv-types.rs#L192

The assertion:

Impossible reg-to-reg copy
UNREACHABLE executed at [...]/rust/src/llvm-project/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp:350

Added a dependent quick fix https://github.com/llvm/llvm-project/commit/37c8d5915f37432f49fd4b0e6ec318ae68750af1.

It appears that this regressed the formatting of initializer lists in some cases:

# Before
% cat ~/test.cc
class A {
  A() : a{} {}

Thank you! This and the follow-up https://reviews.llvm.org/D113958 look reasonable to me.
Letting the people more familiar with the DebugInfo parts of the codebase have a chance to review.

In D113934#3133493, @dexonsmith wrote:

I pushed fa39ec5786c08e78cc247a48c17aef66ca5985fb, a35226b9b606f4ea32e7db9e2f4de0d99ab6bef0, and 7a1b670f0f2f0a38a673b5554bb2c4d9d50447bf to fix most of the blockers. Then https://reviews.llvm.org/D113957 fixes Operation::verify() and https://reviews.llvm.org/D113958 makes it iterate over const Operation and removes the mutable.

Thank you! These look good!

Sorry for submitting quickly, I just wanted to unblock build bots. I still think that this patch is OK considering the weirdness of this Operation class.

I like this. Consider only displaying the major version (e.g., clang-format 14 instead of clang-format 14.0.0).

Thank you!
Let me know if you would like me to commit this for you in case you don't have commit rights for LLVM.

• consider also functions and setters