This is an archive of the discontinued LLVM Phabricator instance.

[Clang] Disable clang-format entirely for clang/test tree.
ClosedPublic

Authored by aaron.ballman on Jun 28 2022, 12:20 AM.

Details

Summary

See discussion here:

https://github.com/llvm/llvm-project/issues/55982

We don't generally expect test files to be formatted according to the
style guide. Indeed, some tests may require specific formatting for the
purposes of the test.

When tests intentionally do not conform to the "correct" formatting,
this causes errors in the CI, which can drown out real errors and causes
people to stop trusting the CI over time.

From the history of the clang/test/.clang-format file, it looks as if
there have been attempts to make clang-format do a subset of formatting
that would be useful for tests. However, it looks as if it's hard to
make clang-format do exactly the right thing -- see the back-and-forth
between
13316a7
and
7b5bddf.

Instead, I think the best choice is to entirely disable clang-format on
the clang/test directory.

Diff Detail

Event Timeline

mboehme created this revision.Jun 28 2022, 12:20 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 28 2022, 12:20 AM
mboehme requested review of this revision.Jun 28 2022, 12:20 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 28 2022, 12:20 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript

The changes here LGTM and I think this is the correct way to go. However, since you discovered that it was perhaps intentional that we did some style checking on tests, do you think we should have a brief community RFC just to make sure everyone's on the same page?

The changes here LGTM and I think this is the correct way to go. However, since you discovered that it was perhaps intentional that we did some style checking on tests, do you think we should have a brief community RFC just to make sure everyone's on the same page?

Good point. Done:

https://discourse.llvm.org/t/rfc-disable-clang-format-in-the-clang-test-tree/63498

Should we also do llvm/test and clang-tools-extra/test at the same time? The LLVM tests will show up in precommit CI, but I'm not certain that precommit CI actually tests clang-tools-extra (it also notably lacks coverage for libc++ and lldb as well).

aaron.ballman commandeered this revision.Jul 1 2022, 7:59 AM
aaron.ballman edited reviewers, added: mboehme; removed: aaron.ballman.

At the off-list request of @mboehme, I'm commandeering this review to take it over the finish line whenever the RFC finishes.

Rebased and made the same changes for the LLVM and clang-tools-extra tests.

Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptJul 5 2022, 7:15 AM
This revision is now accepted and ready to land.Jul 5 2022, 12:38 PM
This revision was landed with ongoing or failed builds.Jul 8 2022, 4:36 AM
This revision was automatically updated to reflect the committed changes.