This is an archive of the discontinued LLVM Phabricator instance.

[String] Apply clang formatting to all string unit tests
ClosedPublic

Authored by ldionne on Dec 23 2022, 12:31 AM.

Details

Summary

In Differential Revision: https://reviews.llvm.org/D140550, I refactored many of the string unit tests which also updated the formatting of the touched files using clang-format. I thought it made sense to apply the formatting to the remainder of the tests in string. If this is not desired, I can delete the PR.

Diff Detail

Event Timeline

bemeryesr created this revision.Dec 23 2022, 12:31 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 23 2022, 12:31 AM
bemeryesr requested review of this revision.Dec 23 2022, 12:31 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 23 2022, 12:31 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
bemeryesr edited the summary of this revision. (Show Details)Dec 23 2022, 12:35 AM
Mordante requested changes to this revision.Dec 23 2022, 9:32 AM
Mordante added a subscriber: Mordante.

Typically we don't want these kind of "format patches". But it looking at the parent patch I think it might be good to make an exception. Since the parent patch is not ready for review yet I mark this patch as needs work. That way it doesn't show up in the review queue. When D140550 is reviewed we can look at this patch. At that time feel free to request a review.

This revision now requires changes to proceed.Dec 23 2022, 9:32 AM
ldionne commandeered this revision.Sep 1 2023, 10:36 AM
ldionne added a reviewer: bemeryesr.
ldionne added a subscriber: ldionne.

Actually I'll first start by clang-formatting the whole thing and then take over D140550. That way D140550 is easier to review.

ldionne updated this revision to Diff 555438.Sep 1 2023, 10:37 AM

Rebase, apply clang-format to all the string tests.

ldionne accepted this revision.Sep 1 2023, 10:38 AM
This revision was not accepted when it landed; it landed in state Needs Review.Sep 1 2023, 10:39 AM
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
phosek added a subscriber: phosek.Sep 1 2023, 3:44 PM

Would it be possible to either revert this change or do a quick fix forward?

libcxx/test/std/strings/basic.string/string.nonmembers/string.io/lit.local.cfg
2–3 ↗(On Diff #555439)

This is not a valid Python syntax, we're seeing the following failure after this change on our builders:

llvm-lit: /b/s/w/ir/x/w/llvm-llvm-project/llvm/utils/lit/lit/TestingConfig.py:151: fatal: unable to parse config file '/b/s/w/ir/x/w/llvm-llvm-project/libcxx/test/std/strings/basic.string/string.nonmembers/string.io/lit.local.cfg', traceback: Traceback (most recent call last):
  File "/b/s/w/ir/x/w/llvm-llvm-project/llvm/utils/lit/lit/TestingConfig.py", line 139, in load_from_path
    exec(compile(data, path, "exec"), cfg_globals, None)
  File "/b/s/w/ir/x/w/llvm-llvm-project/libcxx/test/std/strings/basic.string/string.nonmembers/string.io/lit.local.cfg", line 2
    if
     ^
SyntaxError: invalid syntax