clang-format adds incorrect indentation and trailing whitespace after 4b9764959dc4b8783e18747c1742ab164e4bc4ee

[ilovepi]

After https://reviews.llvm.org/D151954 we're seeing some significant regressions in the output of Clang-Format.
formating_output.zip

The code in this example is generated and formatted w/ Clang-Format and diffed against a golden file (golden.h). To be clear the golden and generated file are both formatted w/ Clang-Format prior to diffing to avoid needlessly detecting irrelevant changes. Our .clang-format file can be found here: https://cs.opensource.google/fuchsia/fuchsia/+/main:.clang-format

As can be seen in bad.h, includes are indented when they shouldn't be and there is whitespace added after lines in many cases. There are also a number of empty lines left in the file.

To illustrate I'm incuding a snippet of the first 30 lines from golden.h and bad.h:

golden.h

// WARNING: This file is machine generated by fidlgen.

// fidl_experiment = no_optional_structs
// fidl_experiment = output_index_json
// fidl_experiment = simple_empty_response_syntax
// fidl_experiment = unknown_interactions

#pragma once

#include <lib/fidl/cpp/wire/array.h>
#include <lib/fidl/cpp/wire/envelope.h>
#include <lib/fidl/cpp/wire/internal/transport_err.h>
#include <lib/fidl/cpp/wire/message.h>
#include <lib/fidl/cpp/wire/message_storage.h>
#include <lib/fidl/cpp/wire/object_view.h>
#include <lib/fidl/cpp/wire/string_view.h>
#include <lib/fidl/cpp/wire/traits.h>
#include <lib/fidl/cpp/wire/wire_types.h>
#include <lib/stdcompat/optional.h>

#include <cinttypes>
#ifdef __Fuchsia__
#include <lib/zx/channel.h>

#endif  // __Fuchsia__

#include <fidl/test.anonymous/cpp/common_types.h>
#include <fidl/test.anonymous/cpp/markers.h>

#pragma clang diagnostic push

bad.h

// WARNING: This file is machine generated by fidlgen.
  
  // fidl_experiment = no_optional_structs
  // fidl_experiment = output_index_json
  // fidl_experiment = simple_empty_response_syntax
  // fidl_experiment = unknown_interactions

  #pragma once

  #include <lib/fidl/cpp/wire/array.h>
  #include <lib/fidl/cpp/wire/envelope.h>
  #include <lib/fidl/cpp/wire/internal/transport_err.h>
  #include <lib/fidl/cpp/wire/message.h>
  #include <lib/fidl/cpp/wire/message_storage.h>
  #include <lib/fidl/cpp/wire/object_view.h>
  #include <lib/fidl/cpp/wire/string_view.h>
  #include <lib/fidl/cpp/wire/traits.h>
  #include <lib/fidl/cpp/wire/wire_types.h>
  #include <lib/stdcompat/optional.h>

  #include <cinttypes>
#ifdef __Fuchsia__
#include <lib/zx/channel.h>
    

#endif  // __Fuchsia__


  #include <fidl/test.anonymous/cpp/common_types.h>
  #include <fidl/test.anonymous/cpp/markers.h>

[llvmbot]

@llvm/issue-subscribers-clang-format

[owenca]

After https://reviews.llvm.org/D151954 we're seeing some significant regressions in the output of Clang-Format. formating_output.zip

The code in this example is generated and formatted w/ Clang-Format and diffed against a golden file (golden.h). To be clear the golden and generated file are both formatted w/ Clang-Format prior to diffing to avoid needlessly detecting irrelevant changes. Our .clang-format file can be found here: https://cs.opensource.google/fuchsia/fuchsia/+/main:.clang-format

What was the latest version did you use to generate your golden.h? I just ran 16.0.4 and 4b97649 with your .clang-format and got exactly the same output:

$ clang-format-16.0.4 golden.h > foo.h
$ clang-format-4b97649 golden.h > bar.h
$ diff foo.h bar.h
$ 

And they are different from your bad.h.

[owenca]

BTW the first 100+ lines are the same:

$ cmp golden.h foo.h
golden.h foo.h differ: char 3004, line 105

[ilovepi]

After https://reviews.llvm.org/D151954 we're seeing some significant regressions in the output of clang-format. formating_output.zip
The code in this example is generated and formatted w/ clang-format and diffed against a golden file (golden.h). To be clear the golden and generated file are both formatted w/ clang-format prior to diffing to avoid needlessly detecting irrelevant changes. Our .clang-format file can be found here: https://cs.opensource.google/fuchsia/fuchsia/+/main:.clang-format

What was the latest version did you use to generate your golden.h? I just ran 16.0.4 and 4b97649 with your .clang-format and got exactly the same output:

I used revision f8b2cbf, which is a bit newer than your patch, but we're having problems with earlier revisions as far back as ac0ea75, which is the first revision to hit our CI w/ your change. (https://ci.chromium.org/ui/p/fuchsia/builders/ci/clang_toolchain.ci.core.arm64-debug/b8779275268692394689/blamelist)

After digging a bit more, it seems like the formatting may have just failed altogether and bad.h is unchanged. I had assumed since it existed that clang-format had done something wrong, but I'm more skeptical of that now. Apologies, since this isn't a part of Fuchsia's build that I'm super familiar with, and I'm going off bot error messages + CI failures to determine what's wrong.

So i think you can reproduce our exact test by

clang-format golden.h > g.h
clang-format bad.h > b.h
diff g.h b.h

And they should come out the same.

Running that locally I'm seeing:

$ byoclang-format ../golden.maybe.h > still_bad.h
The new replacement overlaps with an existing replacement.
New replacement: ../golden.maybe.h: 764:+7:"


"
Existing replacement: ../golden.maybe.h: 764:+7:"

"

Which I think means we're on an error path(

llvm-project/clang/lib/Tooling/Core/Replacement.cpp

Line 172 in 625273e

return "The new replacement overlaps with an existing replacement.";

).

We're not seeing that before your patch. I'm asking some of the folks involved in these tests + infra why we didn't see any error messages, which would have been helpful diagnosing this earlier.

[ilovepi]

also, clang-format apparently exits w/ 0 in this error case, which was a surprise to find out.

[owenca]

I used revision f8b2cbf, which is a bit newer than your patch, but we're having problems with earlier revisions as far back as ac0ea75, which is the first revision to hit our CI w/ your change. (https://ci.chromium.org/ui/p/fuchsia/builders/ci/clang_toolchain.ci.core.arm64-debug/b8779275268692394689/blamelist)

After digging a bit more, it seems like the formatting may have just failed altogether and bad.h is unchanged. I had assumed since it existed that clang-format had done something wrong, but I'm more skeptical of that now. Apologies, since this isn't a part of Fuchsia's build that I'm super familiar with, and I'm going off bot error messages + CI failures to determine what's wrong.

So i think you can reproduce our exact test by

clang-format golden.h > g.h
clang-format bad.h > b.h
diff g.h b.h

And they should come out the same.

I have tested the above with 4b97649, ac0ea75, and f8b2cbf (both Debug and Release builds) and in every case g.h and b.h are the same.

Running that locally I'm seeing:

$ byoclang-format ../golden.maybe.h > still_bad.h
The new replacement overlaps with an existing replacement.
New replacement: ../golden.maybe.h: 764:+7:"


"
Existing replacement: ../golden.maybe.h: 764:+7:"

"

Please attach golden.maybe.h if it's different than golden.h. I don't see this with either golden.h or bad.h.

[owenca]

also, clang-format apparently exits w/ 0 in this error case, which was a surprise to find out.

It has been this way since clang-format 4.

[ilovepi]

So, I'm at a loss. Building at ToT reliably reproduces this for us, and our CI has been broken by this since it landed.

I've managed to reduce the test case down to almost nothing and I can still repro this behavior at just about any revision since your patch landed.

#ifdef 
    

#else 
#endif 

Note there are two ' ' chars on the line after the ifdef.

clearly clang-format should handle this w/o issue, and If I revert your patch locally this no longer happens ...

[ilovepi]

I've supplied a test case in https://reviews.llvm.org/D152473

[owenca]

I've managed to reduce the test case down to almost nothing and I can still repro this behavior at just about any revision since your patch landed.

#ifdef 
    

#else 
#endif 

Note there are two ' ' chars on the line after the ifdef.

I can reproduce the regression with the above. Thanks!

[mydeveloperday]

https://reviews.llvm.org/D152804