This is an archive of the discontinued LLVM Phabricator instance.

Differential D150763

[NFC][Py Reformat] Reformat python files in libcxx/libcxxabi
ClosedPublic

Authored by thieta on May 17 2023, 2:10 AM.

Details

Reviewers
jhenderson
MatzeB
JDevlieghere
ldionne
kwk
Mordante
Group Reviewers
Restricted Project
Restricted Project
Restricted Project
Commits
rG7bfaa0f09d05: [NFC][Py Reformat] Reformat python files in libcxx/libcxxabi
Summary

This is an ongoing series of commits that are reformatting our
Python code.

Reformatting is done with black.

If you end up having problems merging this commit because you
have made changes to a python file, the best way to handle that
is to run git checkout --ours <yourfile> and then reformat it
with black.

If you run into any problems, post to discourse about it and
we will try to help.

RFC Thread below:

https://discourse.llvm.org/t/rfc-document-and-standardize-python-code-style

Diff Detail

Event Timeline

thieta created this revision.May 17 2023, 2:10 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 17 2023, 2:10 AM
thieta requested review of this revision.May 17 2023, 2:10 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptMay 17 2023, 2:10 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
Herald added a reviewer: Restricted Project. · View Herald Transcript
Herald added a subscriber: libcxx-commits. · View Herald Transcript
kwk accepted this revision.May 17 2023, 2:45 AM
kwk added a subscriber: kwk.

LGTM

Harbormaster completed remote builds in B232549: Diff 522960.May 17 2023, 12:09 PM
thieta updated this revision to Diff 523676.May 19 2023, 12:33 AM

Included all .py files as well.

Herald added a project: Restricted Project. · View Herald TranscriptMay 19 2023, 12:33 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
Herald added subscribers: mstorsjo, krytarowski, arichardson. · View Herald Transcript
thieta added subscribers: philnik, Mordante.May 19 2023, 12:34 AM

my initial revision didn't contain all the files. but it should be corrected now. can you have a look @ldionne @Mordante @philnik ?

Harbormaster completed remote builds in B233099: Diff 523676.May 19 2023, 7:01 AM
Mordante added a comment.May 19 2023, 8:47 AM

Another question is it intended to validate the formatting? For libc++ we validate the formatting of our C++ code in the pre-commit CI. I really think it would be great to do that for these files too. If wanted I can do that too.

If you invoked black with command-line arguments could you add them to the commit message?

libcxx/test/libcxx/gdb/gdb_pretty_printer_test.py
7 ↗(On Diff #523676)

This looks odd. Is black set to a line length of 80 or 88 characters? IMO this header part should be the same number as characters as black. I assume it now is 81 characters.

libcxx/utils/generate_feature_test_macro_components.py
9 ↗(On Diff #523676)

I really would appreciate it when this file can remain unformatted for now. This will cause a lot of conflicts with D150795. The other Python files in that commit are smaller. I would be happy to run the formatter afterwards myself.

thieta added inline comments.May 22 2023, 11:38 PM
libcxx/test/libcxx/gdb/gdb_pretty_printer_test.py
7 ↗(On Diff #523676)

Black uses 88. I have another of these headers in clang to fix as well. So I'll see if I can script it after this has landed.

libcxx/utils/generate_feature_test_macro_components.py
9 ↗(On Diff #523676)

I just fixed a bunch of conflicts and it was really easy to just do git checkout <mybranch> <file>; black <file> - so I wouldn't be to worried about conflicts - but if you feel strongly I can exclude it.

thieta updated this revision to Diff 524577.May 22 2023, 11:38 PM

rebase

Harbormaster completed remote builds in B233781: Diff 524577.May 23 2023, 3:27 AM
Mordante accepted this revision.May 24 2023, 10:29 AM

LGTM!

libcxx/test/libcxx/gdb/gdb_pretty_printer_test.py
7 ↗(On Diff #523676)

Sounds good to me.

libcxx/utils/generate_feature_test_macro_components.py
9 ↗(On Diff #523676)

Note D150795 has landed.

thieta updated this revision to Diff 525490.May 25 2023, 2:15 AM

Rebase

This revision was not accepted when it landed; it landed in state Needs Review.May 25 2023, 2:15 AM
This revision was landed with ongoing or failed builds.
Closed by commit rG7bfaa0f09d05: [NFC][Py Reformat] Reformat python files in libcxx/libcxxabi (authored by thieta). · Explain Why
This revision was automatically updated to reflect the committed changes.
thieta added a commit: rG7bfaa0f09d05: [NFC][Py Reformat] Reformat python files in libcxx/libcxxabi.
Harbormaster completed remote builds in B234429: Diff 525490.May 25 2023, 7:37 AM
ldionne added inline comments.Jun 13 2023, 10:41 AM
libcxx/utils/libcxx/test/params.py
208–239 ↗(On Diff #525491)

This was laid out manually in a very intentional manner, and the code is really hard to understand after the reformatting.

Revision Contents

PathSize
libcxx/
test/
libcxx/
experimental/
4 lines
input.output/
file.streams/
5 lines
filesystems/
7 lines
iostream.format/
5 lines
iostream.objects/
5 lines
iostreams.base/
5 lines
stream.buffers/
5 lines
string.streams/
5 lines
3 lines
localization/
5 lines
selftest/
additional_compile_flags/
2 lines
dsl/
3 lines
std/
depr/
depr.ios.members/
4 lines
depr.str.strstreams/
4 lines
experimental/
4 lines
input.output/
file.streams/
8 lines
filesystems/
6 lines
input.output.general/
4 lines
iostream.format/
4 lines
iostream.forward/
4 lines
iostream.objects/
4 lines
iostreams.base/
4 lines
iostreams.requirements/
4 lines
stream.buffers/
4 lines
string.streams/
4 lines
iterators/
stream.iterators/
4 lines
localization/
4 lines
re/
4 lines
strings/
basic.string/
string.nonmembers/
string.io/
4 lines
libcxxabi/
test/
native/
arm-linux-eabi/
7 lines

Diff 522960

libcxx/test/libcxx/experimental/lit.local.cfg

# Disable all of the experimental tests if the correct feature is not available. # Disable all of the experimental tests if the correct feature is not available.
if 'c++experimental' not in config.available_features: if "c++experimental" not in config.available_features:
config.unsupported = True config.unsupported = True

libcxx/test/libcxx/input.output/file.streams/lit.local.cfg

# Load the same local configuration as the corresponding one in libcxx/test/std # Load the same local configuration as the corresponding one in libcxx/test/std
import os import os
inLibcxx = os.path.join('libcxx', 'test', 'libcxx')
inStd = os.path.join('libcxx', 'test', 'std') inLibcxx = os.path.join("libcxx", "test", "libcxx")
inStd = os.path.join("libcxx", "test", "std")
localConfig = os.path.normpath(os.path.realpath(__file__)).replace(inLibcxx, inStd) localConfig = os.path.normpath(os.path.realpath(__file__)).replace(inLibcxx, inStd)
config.load_from_path(localConfig, lit_config) config.load_from_path(localConfig, lit_config)

libcxx/test/libcxx/input.output/filesystems/lit.local.cfg

# Load the same local configuration as the filesystem tests in libcxx/test/std # Load the same local configuration as the filesystem tests in libcxx/test/std
import os import os
std_filesystem_tests = os.path.join(config.test_source_root, 'std', 'input.output', 'filesystems')
config.load_from_path(os.path.join(std_filesystem_tests, 'lit.local.cfg'), lit_config) std_filesystem_tests = os.path.join(
config.test_source_root, "std", "input.output", "filesystems"
)
config.load_from_path(os.path.join(std_filesystem_tests, "lit.local.cfg"), lit_config)

libcxx/test/libcxx/input.output/iostream.format/lit.local.cfg

# Load the same local configuration as the corresponding one in libcxx/test/std # Load the same local configuration as the corresponding one in libcxx/test/std
import os import os
inLibcxx = os.path.join('libcxx', 'test', 'libcxx')
inStd = os.path.join('libcxx', 'test', 'std') inLibcxx = os.path.join("libcxx", "test", "libcxx")
inStd = os.path.join("libcxx", "test", "std")
localConfig = os.path.normpath(os.path.realpath(__file__)).replace(inLibcxx, inStd) localConfig = os.path.normpath(os.path.realpath(__file__)).replace(inLibcxx, inStd)
config.load_from_path(localConfig, lit_config) config.load_from_path(localConfig, lit_config)

libcxx/test/libcxx/input.output/iostream.objects/lit.local.cfg

# Load the same local configuration as the corresponding one in libcxx/test/std # Load the same local configuration as the corresponding one in libcxx/test/std
import os import os
inLibcxx = os.path.join('libcxx', 'test', 'libcxx')
inStd = os.path.join('libcxx', 'test', 'std') inLibcxx = os.path.join("libcxx", "test", "libcxx")
inStd = os.path.join("libcxx", "test", "std")
localConfig = os.path.normpath(os.path.realpath(__file__)).replace(inLibcxx, inStd) localConfig = os.path.normpath(os.path.realpath(__file__)).replace(inLibcxx, inStd)
config.load_from_path(localConfig, lit_config) config.load_from_path(localConfig, lit_config)

libcxx/test/libcxx/input.output/iostreams.base/lit.local.cfg

# Load the same local configuration as the corresponding one in libcxx/test/std # Load the same local configuration as the corresponding one in libcxx/test/std
import os import os
inLibcxx = os.path.join('libcxx', 'test', 'libcxx')
inStd = os.path.join('libcxx', 'test', 'std') inLibcxx = os.path.join("libcxx", "test", "libcxx")
inStd = os.path.join("libcxx", "test", "std")
localConfig = os.path.normpath(os.path.realpath(__file__)).replace(inLibcxx, inStd) localConfig = os.path.normpath(os.path.realpath(__file__)).replace(inLibcxx, inStd)
config.load_from_path(localConfig, lit_config) config.load_from_path(localConfig, lit_config)

libcxx/test/libcxx/input.output/stream.buffers/lit.local.cfg

# Load the same local configuration as the corresponding one in libcxx/test/std # Load the same local configuration as the corresponding one in libcxx/test/std
import os import os
inLibcxx = os.path.join('libcxx', 'test', 'libcxx')
inStd = os.path.join('libcxx', 'test', 'std') inLibcxx = os.path.join("libcxx", "test", "libcxx")
inStd = os.path.join("libcxx", "test", "std")
localConfig = os.path.normpath(os.path.realpath(__file__)).replace(inLibcxx, inStd) localConfig = os.path.normpath(os.path.realpath(__file__)).replace(inLibcxx, inStd)
config.load_from_path(localConfig, lit_config) config.load_from_path(localConfig, lit_config)

libcxx/test/libcxx/input.output/string.streams/lit.local.cfg

# Load the same local configuration as the corresponding one in libcxx/test/std # Load the same local configuration as the corresponding one in libcxx/test/std
import os import os
inLibcxx = os.path.join('libcxx', 'test', 'libcxx')
inStd = os.path.join('libcxx', 'test', 'std') inLibcxx = os.path.join("libcxx", "test", "libcxx")
inStd = os.path.join("libcxx", "test", "std")
localConfig = os.path.normpath(os.path.realpath(__file__)).replace(inLibcxx, inStd) localConfig = os.path.normpath(os.path.realpath(__file__)).replace(inLibcxx, inStd)
config.load_from_path(localConfig, lit_config) config.load_from_path(localConfig, lit_config)

libcxx/test/libcxx/lit.local.cfg

# The tests in this directory need to run Python # The tests in this directory need to run Python
import pipes, sys import pipes, sys
config.substitutions.append(('%{python}', pipes.quote(sys.executable)))
config.substitutions.append(("%{python}", pipes.quote(sys.executable)))

libcxx/test/libcxx/localization/lit.local.cfg

# Load the same local configuration as the corresponding one in libcxx/test/std # Load the same local configuration as the corresponding one in libcxx/test/std
import os import os
inLibcxx = os.path.join('libcxx', 'test', 'libcxx')
inStd = os.path.join('libcxx', 'test', 'std') inLibcxx = os.path.join("libcxx", "test", "libcxx")
inStd = os.path.join("libcxx", "test", "std")
localConfig = os.path.normpath(os.path.realpath(__file__)).replace(inLibcxx, inStd) localConfig = os.path.normpath(os.path.realpath(__file__)).replace(inLibcxx, inStd)
config.load_from_path(localConfig, lit_config) config.load_from_path(localConfig, lit_config)

libcxx/test/libcxx/selftest/additional_compile_flags/lit.local.cfg

# Add a Lit feature so we can test conditional addition of compile flags. # Add a Lit feature so we can test conditional addition of compile flags.
config.available_features.add('some-defined-feature') config.available_features.add("some-defined-feature")

libcxx/test/libcxx/selftest/dsl/lit.local.cfg

# Since we try to pass substitutions as-is to some tests, we must "escape" # Since we try to pass substitutions as-is to some tests, we must "escape"
# them in case they contain other substitutions. Otherwise, the substitutions # them in case they contain other substitutions. Otherwise, the substitutions
# will be fully expanded when passed to the tests. For example, we want an # will be fully expanded when passed to the tests. For example, we want an
# %{exec} substitution that contains `--execdir %T` to be passed as-is, without # %{exec} substitution that contains `--execdir %T` to be passed as-is, without
# substituting the directory. This way, the test itself can populate %T as it # substituting the directory. This way, the test itself can populate %T as it
# sees fit, and %{exec} will respect it. # sees fit, and %{exec} will respect it.
# #
# To solve this problem, we pickle the substitutions and base64 encode that # To solve this problem, we pickle the substitutions and base64 encode that
# to pass it to the test, and we decode and unpickle the substitutions from # to pass it to the test, and we decode and unpickle the substitutions from
# within the test. # within the test.
import base64, lit.util, pickle import base64, lit.util, pickle
base64Encode = lambda s: lit.util.to_string(base64.b64encode(lit.util.to_bytes(s))) base64Encode = lambda s: lit.util.to_string(base64.b64encode(lit.util.to_bytes(s)))
escapedSubstitutions = base64Encode(pickle.dumps(config.substitutions)) escapedSubstitutions = base64Encode(pickle.dumps(config.substitutions))
config.substitutions.append(('%{substitutions}', escapedSubstitutions)) config.substitutions.append(("%{substitutions}", escapedSubstitutions))

libcxx/test/std/depr/depr.ios.members/lit.local.cfg

if 'no-localization' in config.available_features: if "no-localization" in config.available_features:
config.unsupported = True config.unsupported = True

libcxx/test/std/depr/depr.str.strstreams/lit.local.cfg

if 'no-localization' in config.available_features: if "no-localization" in config.available_features:
config.unsupported = True config.unsupported = True

libcxx/test/std/experimental/lit.local.cfg

# Disable all of the experimental tests if the correct feature is not available. # Disable all of the experimental tests if the correct feature is not available.
if 'c++experimental' not in config.available_features: if "c++experimental" not in config.available_features:
config.unsupported = True config.unsupported = True

libcxx/test/std/input.output/file.streams/lit.local.cfg

# All non-trivial uses of iostreams require localization support # All non-trivial uses of iostreams require localization support
if 'no-localization' in config.available_features: if "no-localization" in config.available_features:
config.unsupported = True config.unsupported = True
if 'no-fstream' in config.available_features: if "no-fstream" in config.available_features:
config.unsupported = True config.unsupported = True

libcxx/test/std/input.output/filesystems/lit.local.cfg

if 'availability-filesystem-missing' in config.available_features: if "availability-filesystem-missing" in config.available_features:
config.unsupported = True config.unsupported = True
if 'no-filesystem' in config.available_features: if "no-filesystem" in config.available_features:
config.unsupported = True config.unsupported = True

libcxx/test/std/input.output/input.output.general/lit.local.cfg

# All non-trivial uses of iostreams require localization support # All non-trivial uses of iostreams require localization support
if 'no-localization' in config.available_features: if "no-localization" in config.available_features:
config.unsupported = True config.unsupported = True

libcxx/test/std/input.output/iostream.format/lit.local.cfg

# All non-trivial uses of iostreams require localization support # All non-trivial uses of iostreams require localization support
if 'no-localization' in config.available_features: if "no-localization" in config.available_features:
config.unsupported = True config.unsupported = True

libcxx/test/std/input.output/iostream.forward/lit.local.cfg

# All non-trivial uses of iostreams require localization support # All non-trivial uses of iostreams require localization support
if 'no-localization' in config.available_features: if "no-localization" in config.available_features:
config.unsupported = True config.unsupported = True

libcxx/test/std/input.output/iostream.objects/lit.local.cfg

# All non-trivial uses of iostreams require localization support # All non-trivial uses of iostreams require localization support
if 'no-localization' in config.available_features: if "no-localization" in config.available_features:
config.unsupported = True config.unsupported = True

libcxx/test/std/input.output/iostreams.base/lit.local.cfg

# All non-trivial uses of iostreams require localization support # All non-trivial uses of iostreams require localization support
if 'no-localization' in config.available_features: if "no-localization" in config.available_features:
config.unsupported = True config.unsupported = True

libcxx/test/std/input.output/iostreams.requirements/lit.local.cfg

# All non-trivial uses of iostreams require localization support # All non-trivial uses of iostreams require localization support
if 'no-localization' in config.available_features: if "no-localization" in config.available_features:
config.unsupported = True config.unsupported = True

libcxx/test/std/input.output/stream.buffers/lit.local.cfg

# All non-trivial uses of iostreams require localization support # All non-trivial uses of iostreams require localization support
if 'no-localization' in config.available_features: if "no-localization" in config.available_features:
config.unsupported = True config.unsupported = True

libcxx/test/std/input.output/string.streams/lit.local.cfg

# All non-trivial uses of iostreams require localization support # All non-trivial uses of iostreams require localization support
if 'no-localization' in config.available_features: if "no-localization" in config.available_features:
config.unsupported = True config.unsupported = True

libcxx/test/std/iterators/stream.iterators/lit.local.cfg

# stream iterators rely on the streams library, which requires localization # stream iterators rely on the streams library, which requires localization
if 'no-localization' in config.available_features: if "no-localization" in config.available_features:
config.unsupported = True config.unsupported = True

libcxx/test/std/localization/lit.local.cfg

# <locale> tests are obviously not supported when localization support is disabled # <locale> tests are obviously not supported when localization support is disabled
if 'no-localization' in config.available_features: if "no-localization" in config.available_features:
config.unsupported = True config.unsupported = True

libcxx/test/std/re/lit.local.cfg

# Unfortunately, <regex> uses locales in regex_traits # Unfortunately, <regex> uses locales in regex_traits
if 'no-localization' in config.available_features: if "no-localization" in config.available_features:
config.unsupported = True config.unsupported = True

libcxx/test/std/strings/basic.string/string.nonmembers/string.io/lit.local.cfg

# These std::string functions require iostreams, which requires localization # These std::string functions require iostreams, which requires localization
if 'no-localization' in config.available_features: if "no-localization" in config.available_features:
config.unsupported = True config.unsupported = True

libcxxabi/test/native/arm-linux-eabi/lit.local.cfg

def is_arm_linux_eabi(triple): def is_arm_linux_eabi(triple):
return ('arm' in triple) and ('linux' in triple) and ('eabi' in triple) return ("arm" in triple) and ("linux" in triple) and ("eabi" in triple)
is_native = hasattr(config.root, 'target_triple') and (config.root.host_triple == config.root.target_triple)
is_native = hasattr(config.root, "target_triple") and (
config.root.host_triple == config.root.target_triple
)
if not is_native or not is_arm_linux_eabi(config.root.host_triple): if not is_native or not is_arm_linux_eabi(config.root.host_triple):
config.unsupported = True config.unsupported = True