Skip to content

Conversation

@mordante
Copy link
Member

@mordante mordante commented Apr 20, 2024

Adds a new lit directive to improve C++ Standard filtering. This is based on the
Discourse discussion.

mordante added a commit that referenced this pull request May 2, 2024
This is a proof-of-concept how we can test the script. Instead of storing
the data in the script it's stored in a JSON file so a different file can
be used for testing.

This is related to the RFC #89499
mordante added a commit that referenced this pull request Jun 12, 2024
This is a proof-of-concept how we can test the script. Instead of storing
the data in the script it's stored in a JSON file so a different file can
be used for testing.

This is related to the RFC #89499
mordante added a commit that referenced this pull request Jul 4, 2024
This is a proof-of-concept how we can test the script. Instead of storing
the data in the script it's stored in a JSON file so a different file can
be used for testing.

This is related to the RFC #89499
@mordante mordante force-pushed the review/improves_lit_language_version_filtering branch from 96cd69b to 36d446f Compare November 26, 2024 16:31
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.

Personally, I would try to solve the following problems in the following way:

  1. We have a lot of things like UNSUPPORTED: <list of old standards>. Those can be renamed to REQUIRES: std-at-least-c++YZ.
  2. We have some tests that require an older standard, which are currently written as UNSUPPORTED: c++03, c++11, c++14, c++26. I would rewrite those as REQUIRES: c++17 || c++20 || c++23 without introducing a new Lit feature.

IMO this probably provides the most readability.

@mordante mordante force-pushed the review/improves_lit_language_version_filtering branch from 36d446f to cd9debc Compare December 2, 2024 17:36
@mordante mordante changed the title [RFC][libc++][test] Improves C++ Standard filtering. [libc++][test] Improves C++ Standard filtering. Dec 2, 2024
@mordante mordante marked this pull request as ready for review December 2, 2024 17:39
@mordante mordante requested a review from a team as a code owner December 2, 2024 17:39
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Dec 2, 2024
@llvmbot
Copy link
Member

llvmbot commented Dec 2, 2024

@llvm/pr-subscribers-libcxx

Author: Mark de Wever (mordante)

Changes

Adds a new lit directive to improve C++ Standard filtering. This is based on the
Discourse discussion.


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

8 Files Affected:

  • (modified) libcxx/docs/TestingLibcxx.rst (+32)
  • (modified) libcxx/test/std/input.output/iostream.format/print.fun/includes.compile.pass.cpp (+1-1)
  • (modified) libcxx/test/std/input.output/iostream.format/print.fun/no_file_description.pass.cpp (+1-1)
  • (modified) libcxx/test/std/localization/locale.stdcvt/depr.verify.cpp (+1-1)
  • (modified) libcxx/test/std/localization/locales/locale.convenience/conversions/conversions.buffer/depr.verify.cpp (+1-1)
  • (modified) libcxx/test/std/localization/locales/locale.convenience/conversions/conversions.string/depr.verify.cpp (+1-1)
  • (modified) libcxx/test/std/strings/basic.string/string.capacity/reserve.deprecated_in_cxx20.verify.cpp (+1-1)
  • (modified) libcxx/utils/libcxx/test/params.py (+2-1)
diff --git a/libcxx/docs/TestingLibcxx.rst b/libcxx/docs/TestingLibcxx.rst
index 6d2c954489f62e..432d39080049a3 100644
--- a/libcxx/docs/TestingLibcxx.rst
+++ b/libcxx/docs/TestingLibcxx.rst
@@ -435,6 +435,38 @@ writing tests easier. See `libc++-specific Lit Directives`_ for more information
        extension.)
 
 
+C++ Standard version tests
+~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Historically libc++ tests used to filter the tests for C++ Standard versions
+with lit directives like:
+
+.. code-block:: cpp
+
+   // UNSUPPORTED: c++03, c++11, c++14, c++17, c++20, c++23
+
+With C++ Standards released every 3 years, this solution is not scalable.
+Instead use:
+
+.. code-block:: cpp
+
+   // UNSUPPORTED: std-at-least-c++26
+
+There is no corresponding ``std-at-most-c++23``. This could be useful when
+tests are only valid for a small set of standard versions. For example, a
+deprecation test is only valid when the feature is deprecated until it is
+removed from the Standard. These tests should be written like:
+
+.. code-block:: cpp
+
+   // REQUIRES: c++17 || c++20 || c++23
+
+.. note::
+
+   There are a lot of tests with the first style, these can remain as they are.
+   The new style is only intended to be used for new tests.
+
+
 Benchmarks
 ==========
 
diff --git a/libcxx/test/std/input.output/iostream.format/print.fun/includes.compile.pass.cpp b/libcxx/test/std/input.output/iostream.format/print.fun/includes.compile.pass.cpp
index 9b9b0e404e6b7b..38e4e4d3fb9ef5 100644
--- a/libcxx/test/std/input.output/iostream.format/print.fun/includes.compile.pass.cpp
+++ b/libcxx/test/std/input.output/iostream.format/print.fun/includes.compile.pass.cpp
@@ -6,7 +6,7 @@
 //
 //===----------------------------------------------------------------------===//
 
-// UNSUPPORTED: c++03, c++11, c++14, c++17, c++20
+// REQUIRES: std-at-least-c++23
 // UNSUPPORTED: no-filesystem
 // UNSUPPORTED: GCC-ALWAYS_INLINE-FIXME
 
diff --git a/libcxx/test/std/input.output/iostream.format/print.fun/no_file_description.pass.cpp b/libcxx/test/std/input.output/iostream.format/print.fun/no_file_description.pass.cpp
index d3e4463fe0bc89..5561a1a8b33347 100644
--- a/libcxx/test/std/input.output/iostream.format/print.fun/no_file_description.pass.cpp
+++ b/libcxx/test/std/input.output/iostream.format/print.fun/no_file_description.pass.cpp
@@ -6,7 +6,7 @@
 //
 //===----------------------------------------------------------------------===//
 
-// UNSUPPORTED: c++03, c++11, c++14, c++17, c++20
+// REQUIRES: std-at-least-c++23
 // UNSUPPORTED: no-filesystem
 // UNSUPPORTED: GCC-ALWAYS_INLINE-FIXME
 
diff --git a/libcxx/test/std/localization/locale.stdcvt/depr.verify.cpp b/libcxx/test/std/localization/locale.stdcvt/depr.verify.cpp
index b3c6fc8674f8a2..7bdcaa5190bd04 100644
--- a/libcxx/test/std/localization/locale.stdcvt/depr.verify.cpp
+++ b/libcxx/test/std/localization/locale.stdcvt/depr.verify.cpp
@@ -6,7 +6,7 @@
 //
 //===----------------------------------------------------------------------===//
 
-// UNSUPPORTED: c++03, c++11, c++14, c++26
+// REQUIRES: c++17 || c++20 || c++23
 // UNSUPPORTED: no-wide-characters
 
 // <codecvt>
diff --git a/libcxx/test/std/localization/locales/locale.convenience/conversions/conversions.buffer/depr.verify.cpp b/libcxx/test/std/localization/locales/locale.convenience/conversions/conversions.buffer/depr.verify.cpp
index cb067e99a4764b..dcab5cef3a550a 100644
--- a/libcxx/test/std/localization/locales/locale.convenience/conversions/conversions.buffer/depr.verify.cpp
+++ b/libcxx/test/std/localization/locales/locale.convenience/conversions/conversions.buffer/depr.verify.cpp
@@ -6,7 +6,7 @@
 //
 //===----------------------------------------------------------------------===//
 
-// UNSUPPORTED: c++03, c++11, c++14, c++26
+// REQUIRES: c++17 || c++20 || c++23
 
 // XFAIL: no-wide-characters
 
diff --git a/libcxx/test/std/localization/locales/locale.convenience/conversions/conversions.string/depr.verify.cpp b/libcxx/test/std/localization/locales/locale.convenience/conversions/conversions.string/depr.verify.cpp
index f8bd156bdd5f6a..6eab4a5dd9223c 100644
--- a/libcxx/test/std/localization/locales/locale.convenience/conversions/conversions.string/depr.verify.cpp
+++ b/libcxx/test/std/localization/locales/locale.convenience/conversions/conversions.string/depr.verify.cpp
@@ -8,7 +8,7 @@
 
 // ADDITIONAL_COMPILE_FLAGS: -D_LIBCPP_ENABLE_CXX26_REMOVED_WSTRING_CONVERT
 
-// UNSUPPORTED: c++03, c++11, c++14, c++26
+// REQUIRES: c++17 || c++20 || c++23
 // UNSUPPORTED: no-wide-characters
 
 // <codecvt>
diff --git a/libcxx/test/std/strings/basic.string/string.capacity/reserve.deprecated_in_cxx20.verify.cpp b/libcxx/test/std/strings/basic.string/string.capacity/reserve.deprecated_in_cxx20.verify.cpp
index 81edd9b83d184d..87b56c06b95125 100644
--- a/libcxx/test/std/strings/basic.string/string.capacity/reserve.deprecated_in_cxx20.verify.cpp
+++ b/libcxx/test/std/strings/basic.string/string.capacity/reserve.deprecated_in_cxx20.verify.cpp
@@ -10,7 +10,7 @@
 
 // void reserve(); // Deprecated in C++20
 
-// UNSUPPORTED: c++03, c++11, c++14, c++17, c++26
+// REQUIRES: c++20 || c++23
 
 #include <string>
 
diff --git a/libcxx/utils/libcxx/test/params.py b/libcxx/utils/libcxx/test/params.py
index 2c5cb169c0a9a3..626ddd16ebc3ad 100644
--- a/libcxx/utils/libcxx/test/params.py
+++ b/libcxx/utils/libcxx/test/params.py
@@ -186,7 +186,8 @@ def getSuitableClangTidy(cfg):
             AddFeature(std),
             AddSubstitution("%{cxx_std}", re.sub(r"\+", "x", std)),
             AddCompileFlag(lambda cfg: getStdFlag(cfg, std)),
-        ],
+        ]
+        + [AddFeature(f"std-at-least-{s}") for s in _allStandards if s <= std],
     ),
     Parameter(
         name="optimization",

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.

LGTM with small fix.

@ldionne
Copy link
Member

ldionne commented Dec 3, 2024

The documentation build seems to fail (without a good error message).

@ldionne
Copy link
Member

ldionne commented Dec 3, 2024

The documentation build seems to fail (without a good error message).

Nevermind, this is #118555. Let's just wait for that issue to be resolved before we merge this though, so we can at least get a clean documentation build.

This is a proof-of-concept how we could improve the C++ language filtering
in lit. There will be a Discourse post for adding feedback on the
approach.
@mordante mordante force-pushed the review/improves_lit_language_version_filtering branch from a5ca849 to 1f1bfc3 Compare December 23, 2024 16:25
@mordante mordante merged commit de5ff8a into llvm:main Jan 25, 2025
63 checks passed
@mordante mordante deleted the review/improves_lit_language_version_filtering branch January 25, 2025 12:08
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. llvm-lit

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants