[libc++] Fix the declarative generation of FTMs #108843
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We were incorrectly computing whether a FTM has been implemented. Instead of checking whether any version of the FTM is implemented for the current Standard, we need to make sure that the correct version of the FTM has been implemented. As a drive-by fix, also correctly close the file that we load JSON from, which was forgotten.
llvmbot
added
the
libc++
|
@llvm/pr-subscribers-libcxx Author: Louis Dionne (ldionne) ChangesWe were incorrectly computing whether a FTM has been implemented. Instead of checking whether any version of the FTM is implemented for the current Standard, we need to make sure that the correct version of the FTM has been implemented. As a drive-by fix, also correctly close the file that we load JSON from, which was forgotten. Full diff: https://github.com/llvm/llvm-project/pull/108843.diff 7 Files Affected:
diff --git a/libcxx/test/libcxx/feature_test_macro/ftm_metadata.sh.py b/libcxx/test/libcxx/feature_test_macro/ftm_metadata.sh.py
index 1dec1ae612ecb1..8c47448a800415 100644
--- a/libcxx/test/libcxx/feature_test_macro/ftm_metadata.sh.py
+++ b/libcxx/test/libcxx/feature_test_macro/ftm_metadata.sh.py
@@ -47,5 +47,10 @@ def test(output, expected):
"test_suite_guard": None,
"libcxx_guard": None,
},
+ "__cpp_lib_missing_FTM_in_older_standard": {
+ "headers": [],
+ "test_suite_guard": None,
+ "libcxx_guard": None,
+ },
},
)
diff --git a/libcxx/test/libcxx/feature_test_macro/implemented_ftms.sh.py b/libcxx/test/libcxx/feature_test_macro/implemented_ftms.sh.py
index 62a3c46b15db3c..942cd4b6087608 100644
--- a/libcxx/test/libcxx/feature_test_macro/implemented_ftms.sh.py
+++ b/libcxx/test/libcxx/feature_test_macro/implemented_ftms.sh.py
@@ -50,5 +50,10 @@ def test(output, expected):
"c++23": "202102L",
"c++26": "202102L",
},
+ "__cpp_lib_missing_FTM_in_older_standard": {
+ "c++17": None,
+ "c++20": None,
+ "c++26": None,
+ },
},
)
diff --git a/libcxx/test/libcxx/feature_test_macro/standard_ftms.sh.py b/libcxx/test/libcxx/feature_test_macro/standard_ftms.sh.py
index 231e6072898752..95c9b9baaf2fa4 100644
--- a/libcxx/test/libcxx/feature_test_macro/standard_ftms.sh.py
+++ b/libcxx/test/libcxx/feature_test_macro/standard_ftms.sh.py
@@ -50,5 +50,11 @@ def test(output, expected):
"c++23": "202106L",
"c++26": "202306L",
},
+ "__cpp_lib_missing_FTM_in_older_standard": {
+ "c++17": "2017L",
+ "c++20": "2020L",
+ "c++23": "2020L",
+ "c++26": "2026L",
+ },
},
)
diff --git a/libcxx/test/libcxx/feature_test_macro/test_data.json b/libcxx/test/libcxx/feature_test_macro/test_data.json
index 18c8836fa5149f..f7ae74fb83286c 100644
--- a/libcxx/test/libcxx/feature_test_macro/test_data.json
+++ b/libcxx/test/libcxx/feature_test_macro/test_data.json
@@ -154,5 +154,35 @@
"headers": [
"variant"
]
+ },
+ {
+ "name": "__cpp_lib_missing_FTM_in_older_standard",
+ "values": {
+ "c++17": {
+ "2017": [
+ {
+ "title": "Some FTM missing a paper in an older Standard mode, which should result in the FTM never being defined.",
+ "implemented": false
+ }
+ ]
+ },
+ "c++20": {
+ "2020": [
+ {
+ "title": "",
+ "implemented": true
+ }
+ ]
+ },
+ "c++26": {
+ "2026": [
+ {
+ "title": "",
+ "implemented": true
+ }
+ ]
+ }
+ },
+ "headers": []
}
]
diff --git a/libcxx/test/libcxx/feature_test_macro/version_header.sh.py b/libcxx/test/libcxx/feature_test_macro/version_header.sh.py
index bbb863371f7639..634f43b83cafaa 100644
--- a/libcxx/test/libcxx/feature_test_macro/version_header.sh.py
+++ b/libcxx/test/libcxx/feature_test_macro/version_header.sh.py
@@ -43,6 +43,7 @@ def test(output, expected):
# define __cpp_lib_any 201606L
# define __cpp_lib_parallel_algorithm 201603L
# define __cpp_lib_variant 202102L
+// define __cpp_lib_missing_FTM_in_older_standard 2017L
#endif // _LIBCPP_STD_VER >= 17
#if _LIBCPP_STD_VER >= 20
@@ -50,8 +51,8 @@ def test(output, expected):
# define __cpp_lib_barrier 201907L
# endif
// define __cpp_lib_format 202110L
-# undef __cpp_lib_variant
-# define __cpp_lib_variant 202106L
+// define __cpp_lib_variant 202106L
+// define __cpp_lib_missing_FTM_in_older_standard 2020L
#endif // _LIBCPP_STD_VER >= 20
#if _LIBCPP_STD_VER >= 23
@@ -64,8 +65,8 @@ def test(output, expected):
# define __cpp_lib_barrier 299900L
# endif
// define __cpp_lib_format 202311L
-# undef __cpp_lib_variant
-# define __cpp_lib_variant 202306L
+// define __cpp_lib_variant 202306L
+// define __cpp_lib_missing_FTM_in_older_standard 2026L
#endif // _LIBCPP_STD_VER >= 26
#endif // _LIBCPP_VERSION
diff --git a/libcxx/test/libcxx/feature_test_macro/version_header_implementation.sh.py b/libcxx/test/libcxx/feature_test_macro/version_header_implementation.sh.py
index db90206ae770f3..a2fc1c5682f63b 100644
--- a/libcxx/test/libcxx/feature_test_macro/version_header_implementation.sh.py
+++ b/libcxx/test/libcxx/feature_test_macro/version_header_implementation.sh.py
@@ -9,106 +9,137 @@
# RUN: %{python} %s %{libcxx-dir}/utils %{libcxx-dir}/test/libcxx/feature_test_macro/test_data.json
import sys
+import unittest
-sys.path.append(sys.argv[1])
-from generate_feature_test_macro_components import FeatureTestMacros
-
+UTILS = sys.argv[1]
+TEST_DATA = sys.argv[2]
+del sys.argv[1:3]
-def test(output, expected):
- assert output == expected, f"expected\n{expected}\n\noutput\n{output}"
+sys.path.append(UTILS)
+from generate_feature_test_macro_components import FeatureTestMacros
+class Test(unittest.TestCase):
+ def setUp(self):
+ self.ftm = FeatureTestMacros(TEST_DATA)
+ self.maxDiff = None # This causes the diff to be printed when the test fails
-ftm = FeatureTestMacros(sys.argv[2])
-test(
- ftm.version_header_implementation,
- {
- "17": [
- {
- "__cpp_lib_any": {
- "value": "201606L",
- "implemented": True,
- "need_undef": False,
- "condition": None,
+ def test_implementation(self):
+ expected = {
+ "17": [
+ {
+ "__cpp_lib_any": {
+ "value": "201606L",
+ "implemented": True,
+ "need_undef": False,
+ "condition": None,
+ },
+ },
+ {
+ "__cpp_lib_parallel_algorithm": {
+ "value": "201603L",
+ "implemented": True,
+ "need_undef": False,
+ "condition": None,
+ },
},
- },
- {
- "__cpp_lib_parallel_algorithm": {
- "value": "201603L",
- "implemented": True,
- "need_undef": False,
- "condition": None,
+ {
+ "__cpp_lib_variant": {
+ "value": "202102L",
+ "implemented": True,
+ "need_undef": False,
+ "condition": None,
+ },
},
- },
- {
- "__cpp_lib_variant": {
- "value": "202102L",
- "implemented": True,
- "need_undef": False,
- "condition": None,
+ {
+ "__cpp_lib_missing_FTM_in_older_standard": {
+ "value": "2017L",
+ "implemented": False,
+ "need_undef": False,
+ "condition": None,
+ },
},
- },
- ],
- "20": [
- {
- "__cpp_lib_barrier": {
- "value": "201907L",
- "implemented": True,
- "need_undef": False,
- "condition": "!defined(_LIBCPP_HAS_NO_THREADS) && _LIBCPP_AVAILABILITY_HAS_SYNC",
+ ],
+ "20": [
+ {
+ "__cpp_lib_barrier": {
+ "value": "201907L",
+ "implemented": True,
+ "need_undef": False,
+ "condition": "!defined(_LIBCPP_HAS_NO_THREADS) && _LIBCPP_AVAILABILITY_HAS_SYNC",
+ },
},
- },
- {
- "__cpp_lib_format": {
- "value": "202110L",
- "implemented": False,
- "need_undef": False,
- "condition": None,
+ {
+ "__cpp_lib_format": {
+ "value": "202110L",
+ "implemented": False,
+ "need_undef": False,
+ "condition": None,
+ },
},
- },
- {
- "__cpp_lib_variant": {
- "value": "202106L",
- "implemented": True,
- "need_undef": True,
- "condition": None,
+ {
+ "__cpp_lib_variant": {
+ "value": "202106L",
+ "implemented": False,
+ "need_undef": False,
+ "condition": None,
+ },
},
- },
- ],
- "23": [
- {
- "__cpp_lib_format": {
- "value": "202207L",
- "implemented": False,
- "need_undef": False,
- "condition": None,
+ {
+ "__cpp_lib_missing_FTM_in_older_standard": {
+ "value": "2020L",
+ "implemented": False,
+ "need_undef": False,
+ "condition": None,
+ },
},
- },
- ],
- "26": [
- {
- "__cpp_lib_barrier": {
- "value": "299900L",
- "implemented": True,
- "need_undef": True,
- "condition": "!defined(_LIBCPP_HAS_NO_THREADS) && _LIBCPP_AVAILABILITY_HAS_SYNC",
+ ],
+ "23": [
+ {
+ "__cpp_lib_format": {
+ "value": "202207L",
+ "implemented": False,
+ "need_undef": False,
+ "condition": None,
+ },
},
- },
- {
- "__cpp_lib_format": {
- "value": "202311L",
- "implemented": False,
- "need_undef": False,
- "condition": None,
+ ],
+ "26": [
+ {
+ "__cpp_lib_barrier": {
+ "value": "299900L",
+ "implemented": True,
+ "need_undef": True,
+ "condition": "!defined(_LIBCPP_HAS_NO_THREADS) && _LIBCPP_AVAILABILITY_HAS_SYNC",
+ },
},
- },
- {
- "__cpp_lib_variant": {
- "value": "202306L",
- "implemented": True,
- "need_undef": True,
- "condition": None,
+ {
+ "__cpp_lib_format": {
+ "value": "202311L",
+ "implemented": False,
+ "need_undef": False,
+ "condition": None,
+ },
},
- },
- ],
- },
-)
+ {
+ "__cpp_lib_variant": {
+ "value": "202306L",
+ "implemented": False,
+ "need_undef": False,
+ "condition": None,
+ },
+ },
+ {
+ "__cpp_lib_missing_FTM_in_older_standard": {
+ "value": "2026L",
+ "implemented": False,
+ "need_undef": False,
+ "condition": None,
+ },
+ },
+ ],
+ }
+
+ self.assertEqual(self.ftm.version_header_implementation, expected)
+
+if __name__ == '__main__':
+ unittest.main()
diff --git a/libcxx/utils/generate_feature_test_macro_components.py b/libcxx/utils/generate_feature_test_macro_components.py
index cb5ff770b1196e..3b8a52362ede68 100755
--- a/libcxx/utils/generate_feature_test_macro_components.py
+++ b/libcxx/utils/generate_feature_test_macro_components.py
@@ -2102,7 +2102,8 @@ class FeatureTestMacros:
def __init__(self, filename: str):
"""Initializes the class with the JSON data in the file 'filename'."""
- self.__data = json.load(open(filename))
+ with open(filename) as f:
+ self.__data = json.load(f)
@functools.cached_property
def std_dialects(self) -> List[str]:
@@ -2187,8 +2188,8 @@ def version_header_implementation(self) -> Dict[str, List[Dict[str, Any]]]:
result[get_std_number(std)] = list()
for ftm, values in self.standard_ftms.items():
- need_undef = False
last_value = None
+ last_entry = None
for std, value in values.items():
# When a newer Standard does not change the value of the macro
# there is no need to redefine it with the same value.
@@ -2198,12 +2199,11 @@ def version_header_implementation(self) -> Dict[str, List[Dict[str, Any]]]:
entry = dict()
entry["value"] = value
- entry["implemented"] = self.implemented_ftms[ftm][std] != None
- entry["need_undef"] = need_undef
+ entry["implemented"] = self.implemented_ftms[ftm][std] == self.standard_ftms[ftm][std]
+ entry["need_undef"] = last_entry is not None and last_entry["implemented"] and entry["implemented"]
entry["condition"] = self.ftm_metadata[ftm]["libcxx_guard"]
- need_undef = entry["implemented"]
-
+ last_entry = entry
result[get_std_number(std)].append(dict({ftm: entry}))
return result
|
You can test this locally with the following command:darker --check --diff -r 165f0e80f6b811ab0c82b52e39cb5fb4010850b7...2cefd1eaf52cdd98647f4d17bb06d467b62af2c1 libcxx/test/libcxx/feature_test_macro/ftm_metadata.sh.py libcxx/test/libcxx/feature_test_macro/implemented_ftms.sh.py libcxx/test/libcxx/feature_test_macro/standard_ftms.sh.py libcxx/test/libcxx/feature_test_macro/version_header.sh.py libcxx/test/libcxx/feature_test_macro/version_header_implementation.sh.py libcxx/utils/generate_feature_test_macro_components.pyView the diff from darker here.--- test/libcxx/feature_test_macro/version_header_implementation.sh.py 2024-09-16 15:17:32.000000 +0000
+++ test/libcxx/feature_test_macro/version_header_implementation.sh.py 2024-09-16 15:27:08.315569 +0000
@@ -16,14 +16,15 @@
del sys.argv[1:3]
sys.path.append(UTILS)
from generate_feature_test_macro_components import FeatureTestMacros
+
class Test(unittest.TestCase):
def setUp(self):
self.ftm = FeatureTestMacros(TEST_DATA)
- self.maxDiff = None # This causes the diff to be printed when the test fails
+ self.maxDiff = None # This causes the diff to be printed when the test fails
def test_implementation(self):
expected = {
"17": [
{
@@ -139,7 +140,8 @@
],
}
self.assertEqual(self.ftm.version_header_implementation, expected)
-if __name__ == '__main__':
+
+if __name__ == "__main__":
unittest.main()
--- utils/generate_feature_test_macro_components.py 2024-09-16 15:17:32.000000 +0000
+++ utils/generate_feature_test_macro_components.py 2024-09-16 15:27:08.934662 +0000
@@ -2197,12 +2197,18 @@
continue
last_value = value
entry = dict()
entry["value"] = value
- entry["implemented"] = self.implemented_ftms[ftm][std] == self.standard_ftms[ftm][std]
- entry["need_undef"] = last_entry is not None and last_entry["implemented"] and entry["implemented"]
+ entry["implemented"] = (
+ self.implemented_ftms[ftm][std] == self.standard_ftms[ftm][std]
+ )
+ entry["need_undef"] = (
+ last_entry is not None
+ and last_entry["implemented"]
+ and entry["implemented"]
+ )
entry["condition"] = self.ftm_metadata[ftm]["libcxx_guard"]
last_entry = entry
result[get_std_number(std)].append(dict({ftm: entry}))
|
ldionne
commented
Sep 17, 2024
| #endif // _LIBCPP_STD_VER >= 17 | ||
|
|
||
| #if _LIBCPP_STD_VER >= 20 | ||
| # if !defined(_LIBCPP_HAS_NO_THREADS) && _LIBCPP_AVAILABILITY_HAS_SYNC | ||
| # define __cpp_lib_barrier 201907L | ||
| # endif | ||
| // define __cpp_lib_format 202110L | ||
| # undef __cpp_lib_variant | ||
| # define __cpp_lib_variant 202106L | ||
| // define __cpp_lib_variant 202106L |
There was a problem hiding this comment.
@cpplearner I think that is what your comment was about. Does this make more sense?
hamphet
pushed a commit
to hamphet/llvm-project
that referenced
this pull request
Sep 18, 2024
We were incorrectly computing whether a FTM has been implemented. Instead of checking whether any version of the FTM is implemented for the current Standard, we need to make sure that the correct version of the FTM has been implemented. As a drive-by fix, also correctly close the file that we load JSON from, which was forgotten.
tmsri
pushed a commit
to tmsri/llvm-project
that referenced
this pull request
Sep 19, 2024
We were incorrectly computing whether a FTM has been implemented. Instead of checking whether any version of the FTM is implemented for the current Standard, we need to make sure that the correct version of the FTM has been implemented. As a drive-by fix, also correctly close the file that we load JSON from, which was forgotten.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We were incorrectly computing whether a FTM has been implemented. Instead of checking whether any version of the FTM is implemented for the current Standard, we need to make sure that the correct version of the FTM has been implemented.
As a drive-by fix, also correctly close the file that we load JSON from, which was forgotten.