Skip to content

[libc++][test] Adds backdeployment shorthands. #78204

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 9, 2024

Conversation

mordante
Copy link
Member

@mordante mordante commented Jan 15, 2024

Some changes in libc++ affect the dylib. These changes are not present on systems that use the system dylib. Currently that are the Apple backdeployment targets. Figuring out which MacOS versions to target is not trivial for non-Apple engineers. These shorthands make it easier to select the proper feature make a test UNSUPPORTED or XFAIL.

During the design discussion with Louis we considered whether or not to add preprocessor definitions to allow partial disabling of a test. This would be useful when an existing feature is changed by modifying the dylib. In the end we decided not to add this feature to avoid additional complexity in the tests. Instead the test will be disabled for that target.

Copy link

github-actions bot commented Jan 15, 2024

⚠️ Python code formatter, darker found issues in your code. ⚠️

You can test this locally with the following command:
darker --check --diff -r 5ca2777c69f8708d583e230c56ac7f5f6376fb40...9328c1b776eac57ddb3cd9abb050541c7337a6aa libcxx/utils/libcxx/test/features.py
View the diff from darker here.
--- features.py	2024-02-03 18:42:41.000000 +0000
+++ features.py	2024-02-03 18:45:18.160312 +0000
@@ -531,121 +531,113 @@
     Feature(
         name="using-built-library-before-llvm-11",
         when=lambda cfg: BooleanExpression.evaluate(
             "stdlib=apple-libc++ && target={{.+}}-apple-macosx{{(10.9|10.10|10.11|10.12|10.13|10.14|10.15|11.0)(.0)?}}",
             cfg.available_features,
-        )
+        ),
     ),
     Feature(
         name="using-built-library-before-llvm-12",
         when=lambda cfg: BooleanExpression.evaluate(
             "using-built-library-before-llvm-11 || (stdlib=apple-libc++ && target={{.+}}-apple-macosx12.{{(0|1|2)}}.0)",
             cfg.available_features,
-        )
-    ),
-
+        ),
+    ),
     Feature(
         name="using-built-library-before-llvm-13",
         when=lambda cfg: BooleanExpression.evaluate(
             "using-built-library-before-llvm-12 || (stdlib=apple-libc++ && target={{.+}}-apple-macosx{{((12.(3|4|5|6|7))|(13.(0|1|2|3)))}}.0)",
             cfg.available_features,
-        )
-    ),
-
+        ),
+    ),
     Feature(
         name="using-built-library-before-llvm-14",
         when=lambda cfg: BooleanExpression.evaluate(
             "using-built-library-before-llvm-13",
             cfg.available_features,
-        )
-    ),
-
+        ),
+    ),
     Feature(
         name="using-built-library-before-llvm-15",
         when=lambda cfg: BooleanExpression.evaluate(
             "using-built-library-before-llvm-14 || (stdlib=apple-libc++ && target={{.+}}-apple-macosx13.{{(4|5|6)}}.0)",
             cfg.available_features,
-        )
-    ),
-
+        ),
+    ),
     Feature(
         name="using-built-library-before-llvm-16",
         when=lambda cfg: BooleanExpression.evaluate(
             "using-built-library-before-llvm-15 || (stdlib=apple-libc++ && target={{.+}}-apple-macosx14.{{(0|1|2|3)}}.0)",
             cfg.available_features,
-        )
-    ),
-
+        ),
+    ),
     Feature(
         name="using-built-library-before-llvm-17",
         when=lambda cfg: BooleanExpression.evaluate(
             "using-built-library-before-llvm-16",
             cfg.available_features,
-        )
-    ),
-
+        ),
+    ),
     Feature(
         name="using-built-library-before-llvm-18",
         when=lambda cfg: BooleanExpression.evaluate(
             # For now, no released version of macOS contains LLVM 18
             # TODO(ldionne) Please provide the correct value.
             "using-built-library-before-llvm-17 || stdlib=apple-libc++ && target={{.+}}-apple-macosx{{.+}}",
             cfg.available_features,
-        )
-    ),
-
+        ),
+    ),
     Feature(
         name="using-built-library-before-llvm-19",
         when=lambda cfg: BooleanExpression.evaluate(
             # For now, no released version of macOS contains LLVM 19
             # TODO(ldionne) Please provide the correct value.
             "using-built-library-before-llvm-18 || stdlib=apple-libc++ && target={{.+}}-apple-macosx{{.+}}",
             cfg.available_features,
-        )
-    ),
-
+        ),
+    ),
     # Tests that require std::to_chars(floating-point) in the built library
     Feature(
         name="availability-fp_to_chars-missing",
         when=lambda cfg: BooleanExpression.evaluate(
-            #"stdlib=apple-libc++ && target={{.+}}-apple-macosx{{(10.13|10.14|10.15|11.0|12.0|13.0)(.0)?}}",
+            # "stdlib=apple-libc++ && target={{.+}}-apple-macosx{{(10.13|10.14|10.15|11.0|12.0|13.0)(.0)?}}",
             "using-built-library-before-llvm-13",
             cfg.available_features,
         ),
     ),
     # Tests that require https://wg21.link/P0482 support in the built library
     Feature(
         name="availability-char8_t_support-missing",
         when=lambda cfg: BooleanExpression.evaluate(
-            #"stdlib=apple-libc++ && target={{.+}}-apple-macosx{{(10.13|10.14|10.15|11.0)(.0)?}}",
+            # "stdlib=apple-libc++ && target={{.+}}-apple-macosx{{(10.13|10.14|10.15|11.0)(.0)?}}",
             "using-built-library-before-llvm-11",
             cfg.available_features,
         ),
     ),
     # Tests that require __libcpp_verbose_abort support in the built library
     Feature(
         name="availability-verbose_abort-missing",
         when=lambda cfg: BooleanExpression.evaluate(
-            #"stdlib=apple-libc++ && target={{.+}}-apple-macosx{{(10.13|10.14|10.15|11.0|12.0|13.0)(.0)?}}",
+            # "stdlib=apple-libc++ && target={{.+}}-apple-macosx{{(10.13|10.14|10.15|11.0|12.0|13.0)(.0)?}}",
             "using-built-library-before-llvm-13",
             cfg.available_features,
         ),
     ),
     # Tests that require std::pmr support in the built library
     Feature(
         name="availability-pmr-missing",
         when=lambda cfg: BooleanExpression.evaluate(
-            #"stdlib=apple-libc++ && target={{.+}}-apple-macosx{{(10.13|10.14|10.15|11.0|12.0|13.0)(.0)?}}",
+            # "stdlib=apple-libc++ && target={{.+}}-apple-macosx{{(10.13|10.14|10.15|11.0|12.0|13.0)(.0)?}}",
             "using-built-library-before-llvm-13",
             cfg.available_features,
         ),
     ),
     # Tests that require std::filesystem support in the built library
     Feature(
         name="availability-filesystem-missing",
         when=lambda cfg: BooleanExpression.evaluate(
-            #"stdlib=apple-libc++ && target={{.+}}-apple-macosx10.{{(13|14)(.0)?}}",
+            # "stdlib=apple-libc++ && target={{.+}}-apple-macosx10.{{(13|14)(.0)?}}",
             "using-built-library-before-llvm-16",
             cfg.available_features,
         ),
     ),
     # Tests that require the C++20 synchronization library (P1135R6 implemented by https://llvm.org/D68480) in the built library
@@ -658,20 +650,20 @@
     ),
     # Tests that require time zone database support in the built library
     Feature(
         name="availability-tzdb-missing",
         when=lambda cfg: BooleanExpression.evaluate(
-            #"(stdlib=apple-libc++ && target={{.+}}-apple-macosx{{(10.13|10.14|10.15|11.0|12.0|13.0)(.0)?}})",
+            # "(stdlib=apple-libc++ && target={{.+}}-apple-macosx{{(10.13|10.14|10.15|11.0|12.0|13.0)(.0)?}})",
             "using-built-library-before-llvm-16",
             cfg.available_features,
         ),
     ),
     # Tests that require support for <print> and std::print in <ostream> in the built library.
     Feature(
         name="availability-print-missing",
         when=lambda cfg: BooleanExpression.evaluate(
-            #"stdlib=apple-libc++ && target={{.+}}-apple-macosx{{(10.9|10.10|10.11|10.12|10.13|10.14|10.15|11.0|12.0|13.0)(.0)?}}",
+            # "stdlib=apple-libc++ && target={{.+}}-apple-macosx{{(10.9|10.10|10.11|10.12|10.13|10.14|10.15|11.0|12.0|13.0)(.0)?}}",
             "using-built-library-before-llvm-18",
             cfg.available_features,
         ),
     ),
 ]

@mordante mordante force-pushed the test_backdeployment_short_hands branch 2 times, most recently from 9328c1b to ed3ede3 Compare February 4, 2024 10:55
Some changes in libc++ affect the dylib. These changes are not present
on systems that use the system dylib. Currently that are the Apple
backdeployment targets. Figuring out which MacOS versions to target is
not trivial for non-Apple engineers. These shorthands make it easier to
select the proper feature make a test UNSUPPORTED or XFAIL.

During the design discussion with @ldionne we considered whether or not
to add preprocessor definitions to allow partial disabling of a test.
This would be useful when an existing feature is changed by modifying
the dylib. In the end we decided not to add this feature to avoid
additional complexity in the tests. Instead the test will be disabled
for that target.
@mordante mordante force-pushed the test_backdeployment_short_hands branch from ed3ede3 to 133e663 Compare February 4, 2024 13:38
@mordante mordante marked this pull request as ready for review February 4, 2024 17:38
@mordante mordante requested a review from a team as a code owner February 4, 2024 17:38
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Feb 4, 2024
@llvmbot
Copy link
Member

llvmbot commented Feb 4, 2024

@llvm/pr-subscribers-libcxx

Author: Mark de Wever (mordante)

Changes

Some changes in libc++ affect the dylib. These changes are not present on systems that use the system dylib. Currently that are the Apple backdeployment targets. Figuring out which MacOS versions to target is not trivial for non-Apple engineers. These shorthands make it easier to select the proper feature make a test UNSUPPORTED or XFAIL.

During the design discussion with @ldionne we considered whether or not to add preprocessor definitions to allow partial disabling of a test. This would be useful when an existing feature is changed by modifying the dylib. In the end we decided not to add this feature to avoid additional complexity in the tests. Instead the test will be disabled for that target.


Full diff: https://github.com/llvm/llvm-project/pull/78204.diff

1 Files Affected:

  • (modified) libcxx/utils/libcxx/test/features.py (+95-6)
diff --git a/libcxx/utils/libcxx/test/features.py b/libcxx/utils/libcxx/test/features.py
index ae719a1d47457..a9fb64a8ce190 100644
--- a/libcxx/utils/libcxx/test/features.py
+++ b/libcxx/utils/libcxx/test/features.py
@@ -526,12 +526,94 @@ def check_gdb(cfg):
 # target that doesn't support it will fail at compile time, not at runtime. This can
 # be achieved by creating a `.verify.cpp` test that checks for the right errors, and
 # mark that test as requiring `stdlib=<vendor>-libc++ && target=<target>`.
+#
+# Since it is not always known which deployment target to pick there are
+# short-hands based on the LLVM version like using-built-library-before-llvm-xx.
+# These short-hands make it easy for libc++ developers to select the proper
+# version the feature will be available in and allows vendors to set the proper
+# target information.
 DEFAULT_FEATURES += [
+    # Backdeployment short-hands
+    Feature(
+        name="using-built-library-before-llvm-11",
+        when=lambda cfg: BooleanExpression.evaluate(
+            "stdlib=apple-libc++ && target={{.+}}-apple-macosx{{(10.9|10.10|10.11|10.12|10.13|10.14|10.15|11.0)(.0)?}}",
+            cfg.available_features,
+        ),
+    ),
+    Feature(
+        name="using-built-library-before-llvm-12",
+        when=lambda cfg: BooleanExpression.evaluate(
+            "using-built-library-before-llvm-11 || (stdlib=apple-libc++ && target={{.+}}-apple-macosx12.{{(0|1|2)}}.0)",
+            cfg.available_features,
+        ),
+    ),
+
+    Feature(
+        name="using-built-library-before-llvm-13",
+        when=lambda cfg: BooleanExpression.evaluate(
+            "using-built-library-before-llvm-12 || (stdlib=apple-libc++ && target={{.+}}-apple-macosx{{((12.(3|4|5|6|7))|(13.(0|1|2|3)))}}.0)",
+            cfg.available_features,
+        ),
+    ),
+
+    Feature(
+        name="using-built-library-before-llvm-14",
+        when=lambda cfg: BooleanExpression.evaluate(
+            "using-built-library-before-llvm-13",
+            cfg.available_features,
+        ),
+    ),
+
+    Feature(
+        name="using-built-library-before-llvm-15",
+        when=lambda cfg: BooleanExpression.evaluate(
+            "using-built-library-before-llvm-14 || (stdlib=apple-libc++ && target={{.+}}-apple-macosx13.{{(4|5|6)}}.0)",
+            cfg.available_features,
+        ),
+    ),
+
+    Feature(
+        name="using-built-library-before-llvm-16",
+        when=lambda cfg: BooleanExpression.evaluate(
+            "using-built-library-before-llvm-15 || (stdlib=apple-libc++ && target={{.+}}-apple-macosx14.{{(0|1|2|3)}}.0)",
+            cfg.available_features,
+        ),
+    ),
+
+    Feature(
+        name="using-built-library-before-llvm-17",
+        when=lambda cfg: BooleanExpression.evaluate(
+            "using-built-library-before-llvm-16",
+            cfg.available_features,
+        ),
+    ),
+
+    Feature(
+        name="using-built-library-before-llvm-18",
+        when=lambda cfg: BooleanExpression.evaluate(
+            # For now, no released version of macOS contains LLVM 18
+            # TODO(ldionne) Please provide the correct value.
+            "using-built-library-before-llvm-17 || stdlib=apple-libc++ && target={{.+}}-apple-macosx{{.+}}",
+            cfg.available_features,
+        ),
+    ),
+
+    Feature(
+        name="using-built-library-before-llvm-19",
+        when=lambda cfg: BooleanExpression.evaluate(
+            # For now, no released version of macOS contains LLVM 19
+            # TODO(ldionne) Please provide the correct value.
+            "using-built-library-before-llvm-18 || stdlib=apple-libc++ && target={{.+}}-apple-macosx{{.+}}",
+            cfg.available_features,
+        ),
+    ),
+
     # Tests that require std::to_chars(floating-point) in the built library
     Feature(
         name="availability-fp_to_chars-missing",
         when=lambda cfg: BooleanExpression.evaluate(
-            "stdlib=apple-libc++ && target={{.+}}-apple-macosx{{(10.13|10.14|10.15|11.0|12.0|13.0)(.0)?}}",
+            "using-built-library-before-llvm-13",
             cfg.available_features,
         ),
     ),
@@ -539,7 +621,7 @@ def check_gdb(cfg):
     Feature(
         name="availability-char8_t_support-missing",
         when=lambda cfg: BooleanExpression.evaluate(
-            "stdlib=apple-libc++ && target={{.+}}-apple-macosx{{(10.13|10.14|10.15|11.0)(.0)?}}",
+            "using-built-library-before-llvm-11",
             cfg.available_features,
         ),
     ),
@@ -547,7 +629,7 @@ def check_gdb(cfg):
     Feature(
         name="availability-verbose_abort-missing",
         when=lambda cfg: BooleanExpression.evaluate(
-            "stdlib=apple-libc++ && target={{.+}}-apple-macosx{{(10.13|10.14|10.15|11.0|12.0|13.0)(.0)?}}",
+            "using-built-library-before-llvm-13",
             cfg.available_features,
         ),
     ),
@@ -555,7 +637,7 @@ def check_gdb(cfg):
     Feature(
         name="availability-pmr-missing",
         when=lambda cfg: BooleanExpression.evaluate(
-            "stdlib=apple-libc++ && target={{.+}}-apple-macosx{{(10.13|10.14|10.15|11.0|12.0|13.0)(.0)?}}",
+            "using-built-library-before-llvm-13",
             cfg.available_features,
         ),
     ),
@@ -579,8 +661,15 @@ def check_gdb(cfg):
     Feature(
         name="availability-tzdb-missing",
         when=lambda cfg: BooleanExpression.evaluate(
-            # TODO(ldionne) Please provide the correct value.
-            "(stdlib=apple-libc++ && target={{.+}}-apple-macosx{{(10.13|10.14|10.15|11.0|12.0|13.0)(.0)?}})",
+            "using-built-library-before-llvm-19",
+            cfg.available_features,
+        ),
+    ),
+    # Tests that require support for <print> and std::print in <ostream> in the built library.
+    Feature(
+        name="availability-print-missing",
+        when=lambda cfg: BooleanExpression.evaluate(
+            "using-built-library-before-llvm-18",
             cfg.available_features,
         ),
     ),

@ldionne ldionne self-assigned this Feb 6, 2024
Copy link
Member

@ldionne ldionne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a great patch. It really simplifies writing these annotations for upstream folks, and it makes it easier for vendors to maintain them.

As a follow-up, we can probably clean up some ad-hoc stdlib=apple-libc++ && target=... checks in the test suite and replace them with LLVM release short-hands.

@mordante mordante merged commit 4f423e4 into llvm:main Feb 9, 2024
@mordante mordante deleted the test_backdeployment_short_hands branch February 9, 2024 16:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants