[libc++][NFC] Reformat features.py

[ldionne]

Summary

This file was reformatted using the Black tool, which led to entirely
unreadable code due to how lines are broken. Formatting tools are fine,
but not when they lead to code that humans have trouble reading. In the
case of features.py, a lot of it was meant to be aligned in a repetitive
but consistent way to make the structure of the code stand out.
Reformatting with the tool lost that property.

[Mordante]

No objection, but can you provide feedback regarding this issue on Discourse. I expect somebody will suggest enforcing Python formatting on commits; actually quite surprised that wasn't done in this project.

I consider most changes by black not a real improvement.

libcxx/utils/libcxx/test/features.py

26

236

FYI I don't feel these changes by black are bad.

[ldionne]

In D152737#4414916, @Mordante wrote:

No objection, but can you provide feedback regarding this issue on Discourse. I expect somebody will suggest enforcing Python formatting on commits; actually quite surprised that wasn't done in this project.

I consider most changes by black not a real improvement.

Will do. Almost all of the changes I've seen so far have made code much less readable than it used to be. I missed the original discussion but if I had seen it and the result of the reformatting patch, I would have been an over-my-dead-body objection.

I'm in favour of automated formatting in the general case -- we didn't have clang-format and we do now. But when automated formatting makes the code really hard to read for arbitrary restrictions like 89 column width, then I'm not on board anymore.

libcxx/utils/libcxx/test/features.py

 # ===----------------------------------------------------------------------===##
 #
 # Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 # See https://llvm.org/LICENSE.txt for license information.
 # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 #
 # ===----------------------------------------------------------------------===##
 
 from libcxx.test.dsl import *
 from lit.BooleanExpression import BooleanExpression
 import re
 import shutil
 import subprocess
 import sys
 
-_isClang = lambda cfg: "__clang__" in compilerMacros(
-    cfg
-) and "__apple_build_version__" not in compilerMacros(cfg)
+_isClang = lambda cfg: "__clang__" in compilerMacros(cfg) and "__apple_build_version__" not in compilerMacros(cfg)
 _isAppleClang = lambda cfg: "__apple_build_version__" in compilerMacros(cfg)
-_isGCC = lambda cfg: "__GNUC__" in compilerMacros(
-    cfg
-) and "__clang__" not in compilerMacros(cfg)
+_isGCC = lambda cfg: "__GNUC__" in compilerMacros(cfg) and "__clang__" not in compilerMacros(cfg)
 _isMSVC = lambda cfg: "_MSC_VER" in compilerMacros(cfg)
-_msvcVersion = lambda cfg: (
-    int(compilerMacros(cfg)["_MSC_VER"]) // 100,
-    int(compilerMacros(cfg)["_MSC_VER"]) % 100,
-)
+_msvcVersion = lambda cfg: (int(compilerMacros(cfg)["_MSC_VER"]) // 100, int(compilerMacros(cfg)["_MSC_VER"]) % 100)
 
 
 def _getSuitableClangTidy(cfg):
     try:
         # If we didn't build the libcxx-tidy plugin via CMake, we can't run the clang-tidy tests.
-        if (
-            runScriptExitCode(
-                cfg, ["stat %{test-tools}/clang_tidy_checks/libcxx-tidy.plugin"]
-            )
-            != 0
-        ):
+        if runScriptExitCode(cfg, ["stat %{test-tools}/clang_tidy_checks/libcxx-tidy.plugin"])!= 0:
             return None
 
         # TODO MODULES require ToT due module specific fixes.
         if runScriptExitCode(cfg, ['clang-tidy-17 --version']) == 0:
           return 'clang-tidy-17'
 
         # TODO This should be the last stable release.
         # LLVM RELEASE bump to latest stable version
         if runScriptExitCode(cfg, ["clang-tidy-16 --version"]) == 0:
             return "clang-tidy-16"
 
         # LLVM RELEASE bump version
-        if (
-            int(
-                re.search(
-                    "[0-9]+", commandOutput(cfg, ["clang-tidy --version"])
-                ).group()
-            )
-            >= 16
-        ):
+        if int(re.search("[0-9]+", commandOutput(cfg, ["clang-tidy --version"])).group()) >= 16:
             return "clang-tidy"
 
     except ConfigurationRuntimeError:
         return None
 
 
 DEFAULT_FEATURES = [
     Feature(
DEFAULT_FEATURES = [
     #
     # For the corner case of bash existing, but it being missing in the path
     # set in %{exec} as "--env PATH=one-single-dir", the executor does find
     # and executes bash, but bash then can't find any other common shell
     # utilities. Test executing "bash -c 'bash --version'" to see if bash
     # manages to find binaries to execute.
     Feature(
         name="executor-has-no-bash",
-        when=lambda cfg: runScriptExitCode(cfg, ["%{exec} bash -c 'bash --version'"])
-        != 0,
+        when=lambda cfg: runScriptExitCode(cfg, ["%{exec} bash -c 'bash --version'"]) != 0,
     ),
     Feature(
         name="has-clang-tidy",
         when=lambda cfg: _getSuitableClangTidy(cfg) is not None,
         actions=[
             AddSubstitution("%{clang-tidy}", lambda cfg: _getSuitableClangTidy(cfg))
         ],
     ),
     Feature(name="apple-clang", when=_isAppleClang),
     Feature(
         name=lambda cfg: "apple-clang-{__clang_major__}".format(**compilerMacros(cfg)),
         when=_isAppleClang,
     ),
     Feature(
-        name=lambda cfg: "apple-clang-{__clang_major__}.{__clang_minor__}".format(
-            **compilerMacros(cfg)
-        ),
+        name=lambda cfg: "apple-clang-{__clang_major__}.{__clang_minor__}".format(**compilerMacros(cfg)),
         when=_isAppleClang,
     ),
     Feature(
-        name=lambda cfg: "apple-clang-{__clang_major__}.{__clang_minor__}.{__clang_patchlevel__}".format(
-            **compilerMacros(cfg)
-        ),
+        name=lambda cfg: "apple-clang-{__clang_major__}.{__clang_minor__}.{__clang_patchlevel__}".format(**compilerMacros(cfg)),
         when=_isAppleClang,
     ),
     Feature(name="clang", when=_isClang),
     Feature(
         name=lambda cfg: "clang-{__clang_major__}".format(**compilerMacros(cfg)),
         when=_isClang,
     ),
     Feature(
-        name=lambda cfg: "clang-{__clang_major__}.{__clang_minor__}".format(
-            **compilerMacros(cfg)
-        ),
+        name=lambda cfg: "clang-{__clang_major__}.{__clang_minor__}".format(**compilerMacros(cfg)),
         when=_isClang,
     ),
     Feature(
-        name=lambda cfg: "clang-{__clang_major__}.{__clang_minor__}.{__clang_patchlevel__}".format(
-            **compilerMacros(cfg)
-        ),
+        name=lambda cfg: "clang-{__clang_major__}.{__clang_minor__}.{__clang_patchlevel__}".format(**compilerMacros(cfg)),
         when=_isClang,
     ),
     # Note: Due to a GCC bug (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104760), we must disable deprecation warnings
     #       on GCC or spurious diagnostics are issued.
     #
     # TODO:
     # - Enable -Wplacement-new with GCC.
     # - Enable -Wclass-memaccess with GCC.
     Feature(
         name="gcc",
         when=_isGCC,
         actions=[
             AddCompileFlag("-D_LIBCPP_DISABLE_DEPRECATION_WARNINGS"),
             AddCompileFlag("-Wno-placement-new"),
             AddCompileFlag("-Wno-class-memaccess"),
         ],
     ),
     Feature(
         name=lambda cfg: "gcc-{__GNUC__}".format(**compilerMacros(cfg)), when=_isGCC
     ),
     Feature(
-        name=lambda cfg: "gcc-{__GNUC__}.{__GNUC_MINOR__}".format(
-            **compilerMacros(cfg)
-        ),
+        name=lambda cfg: "gcc-{__GNUC__}.{__GNUC_MINOR__}".format(**compilerMacros(cfg)),
         when=_isGCC,
     ),
     Feature(
-        name=lambda cfg: "gcc-{__GNUC__}.{__GNUC_MINOR__}.{__GNUC_PATCHLEVEL__}".format(
-            **compilerMacros(cfg)
-        ),
+        name=lambda cfg: "gcc-{__GNUC__}.{__GNUC_MINOR__}.{__GNUC_PATCHLEVEL__}".format(**compilerMacros(cfg)),
         when=_isGCC,
     ),
     Feature(name="msvc", when=_isMSVC),
     Feature(name=lambda cfg: "msvc-{}".format(*_msvcVersion(cfg)), when=_isMSVC),
     Feature(name=lambda cfg: "msvc-{}.{}".format(*_msvcVersion(cfg)), when=_isMSVC),
 ]
 
 # Deduce and add the test features that that are implied by the #defines in
macros = {
     "_LIBCPP_HAS_NO_LOCALIZATION": "no-localization",
     "_LIBCPP_HAS_NO_WIDE_CHARACTERS": "no-wide-characters",
     "_LIBCPP_HAS_NO_UNICODE": "libcpp-has-no-unicode",
     "_LIBCPP_ENABLE_DEBUG_MODE": "libcpp-has-debug-mode",
 }
 for macro, feature in macros.items():
     DEFAULT_FEATURES.append(
         Feature(
-            name=lambda cfg, m=macro, f=feature: f
-            + ("={}".format(compilerMacros(cfg)[m]) if compilerMacros(cfg)[m] else ""),
+            name=lambda cfg, m=macro, f=feature: f + ("={}".format(compilerMacros(cfg)[m]) if compilerMacros(cfg)[m] else ""),
             when=lambda cfg, m=macro: m in compilerMacros(cfg),
         )
     )
 
 
 # Mapping from canonical locale names (used in the tests) to possible locale
 # names on various systems. Each locale is considered supported if any of the
 # alternative names is supported.