[libc++] Improves type-safety in generator script. #101880
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
llvmbot
added
the
libc++
|
@llvm/pr-subscribers-libcxx Author: Mark de Wever (mordante) ChangesThis changes the code to use dataclasses instead of dict entries. It also adds type aliases to use in the typing information and updates the typing information. Full diff: https://github.com/llvm/llvm-project/pull/101880.diff 3 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 1dec1ae612ecb..f37479490f264 100644
--- a/libcxx/test/libcxx/feature_test_macro/ftm_metadata.sh.py
+++ b/libcxx/test/libcxx/feature_test_macro/ftm_metadata.sh.py
@@ -11,7 +11,7 @@
import sys
sys.path.append(sys.argv[1])
-from generate_feature_test_macro_components import FeatureTestMacros
+from generate_feature_test_macro_components import FeatureTestMacros, Metadata
def test(output, expected):
@@ -19,33 +19,26 @@ def test(output, expected):
ftm = FeatureTestMacros(sys.argv[2])
+
test(
ftm.ftm_metadata,
{
- "__cpp_lib_any": {
- "headers": ["any"],
- "test_suite_guard": None,
- "libcxx_guard": None,
- },
- "__cpp_lib_barrier": {
- "headers": ["barrier"],
- "test_suite_guard": "!defined(_LIBCPP_HAS_NO_THREADS) && (!defined(_LIBCPP_VERSION) || _LIBCPP_AVAILABILITY_HAS_SYNC)",
- "libcxx_guard": "!defined(_LIBCPP_HAS_NO_THREADS) && _LIBCPP_AVAILABILITY_HAS_SYNC",
- },
- "__cpp_lib_format": {
- "headers": ["format"],
- "test_suite_guard": None,
- "libcxx_guard": None,
- },
- "__cpp_lib_parallel_algorithm": {
- "headers": ["algorithm", "numeric"],
- "test_suite_guard": None,
- "libcxx_guard": None,
- },
- "__cpp_lib_variant": {
- "headers": ["variant"],
- "test_suite_guard": None,
- "libcxx_guard": None,
- },
+ "__cpp_lib_any": Metadata(
+ headers=["any"], test_suite_guard=None, libcxx_guard=None
+ ),
+ "__cpp_lib_barrier": Metadata(
+ headers=["barrier"],
+ test_suite_guard="!defined(_LIBCPP_HAS_NO_THREADS) && (!defined(_LIBCPP_VERSION) || _LIBCPP_AVAILABILITY_HAS_SYNC)",
+ libcxx_guard="!defined(_LIBCPP_HAS_NO_THREADS) && _LIBCPP_AVAILABILITY_HAS_SYNC",
+ ),
+ "__cpp_lib_format": Metadata(
+ headers=["format"], test_suite_guard=None, libcxx_guard=None
+ ),
+ "__cpp_lib_parallel_algorithm": Metadata(
+ headers=["algorithm", "numeric"], test_suite_guard=None, libcxx_guard=None
+ ),
+ "__cpp_lib_variant": Metadata(
+ headers=["variant"], test_suite_guard=None, libcxx_guard=None
+ ),
},
)
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 db90206ae770f..2177e150ff4d6 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
@@ -11,7 +11,7 @@
import sys
sys.path.append(sys.argv[1])
-from generate_feature_test_macro_components import FeatureTestMacros
+from generate_feature_test_macro_components import FeatureTestMacros, VersionHeader
def test(output, expected):
@@ -24,90 +24,90 @@ def test(output, expected):
{
"17": [
{
- "__cpp_lib_any": {
- "value": "201606L",
- "implemented": True,
- "need_undef": False,
- "condition": None,
- },
+ "__cpp_lib_any": VersionHeader(
+ 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": VersionHeader(
+ value="201603L",
+ implemented=True,
+ need_undef=False,
+ condition=None,
+ ),
},
{
- "__cpp_lib_variant": {
- "value": "202102L",
- "implemented": True,
- "need_undef": False,
- "condition": None,
- },
+ "__cpp_lib_variant": VersionHeader(
+ value="202102L",
+ implemented=True,
+ 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",
- },
+ "__cpp_lib_barrier": VersionHeader(
+ 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": VersionHeader(
+ value="202110L",
+ implemented=False,
+ need_undef=False,
+ condition=None,
+ ),
},
{
- "__cpp_lib_variant": {
- "value": "202106L",
- "implemented": True,
- "need_undef": True,
- "condition": None,
- },
+ "__cpp_lib_variant": VersionHeader(
+ value="202106L",
+ implemented=True,
+ need_undef=True,
+ condition=None,
+ ),
},
],
"23": [
{
- "__cpp_lib_format": {
- "value": "202207L",
- "implemented": False,
- "need_undef": False,
- "condition": None,
- },
+ "__cpp_lib_format": VersionHeader(
+ value="202207L",
+ 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_barrier": VersionHeader(
+ value="299900L",
+ implemented=True,
+ need_undef=True,
+ condition="!defined(_LIBCPP_HAS_NO_THREADS) && _LIBCPP_AVAILABILITY_HAS_SYNC",
+ ),
},
{
- "__cpp_lib_format": {
- "value": "202311L",
- "implemented": False,
- "need_undef": False,
- "condition": None,
- },
+ "__cpp_lib_format": VersionHeader(
+ value="202311L",
+ implemented=False,
+ need_undef=False,
+ condition=None,
+ ),
},
{
- "__cpp_lib_variant": {
- "value": "202306L",
- "implemented": True,
- "need_undef": True,
- "condition": None,
- },
+ "__cpp_lib_variant": VersionHeader(
+ value="202306L",
+ implemented=True,
+ need_undef=True,
+ condition=None,
+ ),
},
],
},
diff --git a/libcxx/utils/generate_feature_test_macro_components.py b/libcxx/utils/generate_feature_test_macro_components.py
index b041b08f02aac..620c764982e29 100755
--- a/libcxx/utils/generate_feature_test_macro_components.py
+++ b/libcxx/utils/generate_feature_test_macro_components.py
@@ -2,8 +2,15 @@
import os
from builtins import range
+from dataclasses import dataclass
from functools import reduce
-from typing import Any, Dict, List # Needed for python 3.8 compatibility.
+from typing import (
+ Any,
+ Dict,
+ List, # Needed for python 3.8 compatibility.
+ Optional,
+ TypeAlias,
+)
import functools
import json
@@ -1917,9 +1924,29 @@ def produce_docs():
f.write(doc_str)
+Std: TypeAlias = str # Standard version number
+Ftm: TypeAlias = str # The name of a feature test macro
+Value: TypeAlias = str # The value of a feature test macro including the L suffix
+
+
+@dataclass
+class Metadata:
+ headers: list[str] = None
+ test_suite_guard: str = None
+ libcxx_guard: str = None
+
+
+@dataclass
+class VersionHeader:
+ value: Value = None
+ implemented: bool = None
+ need_undef: bool = None
+ condition: str = None
+
+
def get_ftms(
- data, std_dialects: List[str], use_implemented_status: bool
-) -> Dict[str, Dict[str, Any]]:
+ data, std_dialects: List[Std], use_implemented_status: bool
+) -> Dict[Ftm, Dict[Std, Optional[Value]]]:
"""Impementation for FeatureTestMacros.(standard|implemented)_ftms()."""
result = dict()
for feature in data:
@@ -1956,7 +1983,7 @@ def get_ftms(
return result
-def generate_version_header_dialect_block(data: Dict[str, Any]) -> str:
+def generate_version_header_dialect_block(data: Dict[Ftm, VersionHeader]) -> str:
"""Generates the contents of the version header for a dialect.
This generates the contents of a
@@ -1967,27 +1994,29 @@ def generate_version_header_dialect_block(data: Dict[str, Any]) -> str:
result = ""
for element in data:
for ftm, entry in element.items():
- if not entry["implemented"]:
+ if not entry.implemented:
# When a FTM is not implemented don't add the guards
# or undefine the (possibly) defined macro.
- result += f'// define {ftm} {entry["value"]}\n'
+ result += f"// define {ftm} {entry.value}\n"
else:
- need_undef = entry["need_undef"]
- if entry["condition"]:
- result += f'# if {entry["condition"]}\n'
- if entry["need_undef"]:
+ need_undef = entry.need_undef
+ if entry.condition:
+ result += f"# if {entry.condition}\n"
+ if entry.need_undef:
result += f"# undef {ftm}\n"
- result += f'# define {ftm} {entry["value"]}\n'
+ result += f"# define {ftm} {entry.value}\n"
result += f"# endif\n"
else:
- if entry["need_undef"]:
+ if entry.need_undef:
result += f"# undef {ftm}\n"
- result += f'# define {ftm} {entry["value"]}\n'
+ result += f"# define {ftm} {entry.value}\n"
return result
-def generate_version_header_implementation(data: Dict[str, Dict[str, Any]]) -> str:
+def generate_version_header_implementation(
+ data: Dict[Std, Dict[Ftm, VersionHeader]]
+) -> str:
"""Generates the body of the version header."""
template = """#if _LIBCPP_STD_VER >= {dialect}
@@ -2104,7 +2133,7 @@ def __init__(self, filename: str):
self.__data = json.load(open(filename))
@functools.cached_property
- def std_dialects(self) -> List[str]:
+ def std_dialects(self) -> List[Std]:
"""Returns the C++ dialects avaiable.
The available dialects are based on the 'c++xy' keys found the 'values'
@@ -2123,63 +2152,44 @@ def std_dialects(self) -> List[str]:
return sorted(list(dialects))
@functools.cached_property
- def standard_ftms(self) -> Dict[str, Dict[str, Any]]:
+ def standard_ftms(self) -> Dict[Ftm, Dict[Std, Optional[Value]]]:
"""Returns the FTM versions per dialect in the Standard.
This function does not use the 'implemented' flag. The output contains
the versions used in the Standard. When a FTM in libc++ is not
implemented according to the Standard to output may opt to show the
expected value.
-
- The result is a dict with the following content
- - key: Name of the feature test macro.
- - value: A dict with the following content:
- * key: The version of the C++ dialect.
- * value: The value of the feature-test macro.
"""
return get_ftms(self.__data, self.std_dialects, False)
@functools.cached_property
- def implemented_ftms(self) -> Dict[str, Dict[str, Any]]:
+ def implemented_ftms(self) -> Dict[Ftm, Dict[Std, Optional[Value]]]:
"""Returns the FTM versions per dialect implemented in libc++.
Unlike `get_std_dialect_versions` this function uses the 'implemented'
flag. This returns the actual implementation status in libc++.
-
- The result is a dict with the following content
- - key: Name of the feature test macro.
- - value: A dict with the following content:
- * key: The version of the C++ dialect.
- * value: The value of the feature-test macro. When a feature-test
- macro is not implemented its value is None.
"""
return get_ftms(self.__data, self.std_dialects, True)
@functools.cached_property
- def ftm_metadata(self) -> Dict[str, Dict[str, Any]]:
+ def ftm_metadata(self) -> Dict[Ftm, Metadata]:
"""Returns the metadata of the FTMs defined in the Standard.
The metadata does not depend on the C++ dialect used.
- The result is a dict with the following contents:
- - key: Name of the feature test macro.
- - value: A dict with the following content:
- * headers: The list of headers that should provide the FTM
- * test_suite_guard: The condition for testing the FTM in the test suite.
- * test_suite_guard: The condition for testing the FTM in the version header.
"""
result = dict()
for feature in self.__data:
- entry = dict()
- entry["headers"] = feature["headers"]
- entry["test_suite_guard"] = feature.get("test_suite_guard", None)
- entry["libcxx_guard"] = feature.get("libcxx_guard", None)
- result[feature["name"]] = entry
+ result[feature["name"]] = Metadata(
+ feature["headers"],
+ feature.get("test_suite_guard", None),
+ feature.get("libcxx_guard", None),
+ )
return result
@property
- def version_header_implementation(self) -> Dict[str, List[Dict[str, Any]]]:
+ def version_header_implementation(self) -> Dict[Std, Dict[Ftm, VersionHeader]]:
"""Generates the body of the version header."""
result = dict()
for std in self.std_dialects:
@@ -2195,13 +2205,13 @@ def version_header_implementation(self) -> Dict[str, List[Dict[str, Any]]]:
continue
last_value = value
- entry = dict()
- entry["value"] = value
- entry["implemented"] = self.implemented_ftms[ftm][std] != None
- entry["need_undef"] = need_undef
- entry["condition"] = self.ftm_metadata[ftm]["libcxx_guard"]
-
- need_undef = entry["implemented"]
+ entry = VersionHeader(
+ value,
+ self.implemented_ftms[ftm][std] != None,
+ need_undef,
+ self.ftm_metadata[ftm].libcxx_guard,
+ )
+ need_undef = entry.implemented
result[get_std_number(std)].append(dict({ftm: entry}))
|
You can test this locally with the following command:darker --check --diff -r cb1b51f4ff4e2a179dddf492e3310343f53a9ba1...7f2791cac8d697020806a76d156f5ed8cd412910 libcxx/test/libcxx/feature_test_macro/ftm_metadata.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 2025-02-09 17:14:12.000000 +0000
+++ test/libcxx/feature_test_macro/version_header_implementation.sh.py 2025-02-09 17:20:11.273310 +0000
@@ -49,15 +49,15 @@
need_undef=False,
condition=None,
),
},
{
- "__cpp_lib_missing_FTM_in_older_standard" : VersionHeader(
- value = "2017L",
- implemented = False,
- need_undef = False,
- condition = None,
+ "__cpp_lib_missing_FTM_in_older_standard": VersionHeader(
+ value="2017L",
+ implemented=False,
+ need_undef=False,
+ condition=None,
),
},
],
"20": [
{
@@ -83,15 +83,15 @@
need_undef=False,
condition=None,
),
},
{
- "__cpp_lib_missing_FTM_in_older_standard" : VersionHeader(
- value = "2020L",
- implemented = False,
- need_undef = False,
- condition = None,
+ "__cpp_lib_missing_FTM_in_older_standard": VersionHeader(
+ value="2020L",
+ implemented=False,
+ need_undef=False,
+ condition=None,
),
},
],
"23": [
{
@@ -127,15 +127,15 @@
need_undef=False,
condition=None,
),
},
{
- "__cpp_lib_missing_FTM_in_older_standard" : VersionHeader(
- value = "2026L",
- implemented = False,
- need_undef = False,
- condition = None,
+ "__cpp_lib_missing_FTM_in_older_standard": VersionHeader(
+ value="2026L",
+ implemented=False,
+ need_undef=False,
+ condition=None,
),
},
],
}
--- utils/generate_feature_test_macro_components.py 2025-02-09 17:14:12.000000 +0000
+++ utils/generate_feature_test_macro_components.py 2025-02-09 17:20:11.881903 +0000
@@ -1951,11 +1951,14 @@
f.write(doc_str)
Std = NewType("Std", str) # Standard version number
Ftm = NewType("Ftm", str) # The name of a feature test macro
-Value = NewType("Value", str) # The value of a feature test macro including the L suffix
+Value = NewType(
+ "Value", str
+) # The value of a feature test macro including the L suffix
+
@dataclass
class Metadata:
headers: list[str] = None
test_suite_guard: str = None
@@ -2230,11 +2233,13 @@
# there is no need to redefine it with the same value.
if last_value and value == last_value:
continue
last_value = value
- implemented = self.implemented_ftms[ftm][std] == self.standard_ftms[ftm][std]
+ implemented = (
+ self.implemented_ftms[ftm][std] == self.standard_ftms[ftm][std]
+ )
entry = VersionHeader(
value,
implemented,
last_entry is not None and last_entry.implemented and implemented,
self.ftm_metadata[ftm].libcxx_guard,
|
ldionne
approved these changes
Aug 5, 2024
mordante
force-pushed
the
review/generator_dataclasses
branch
from
171b671 to
e7c4f43
Compare
This changes the code to use dataclasses instead of dict entries. It also adds type aliases to use in the typing information and updates the typing information.
mordante
force-pushed
the
review/generator_dataclasses
branch
from
e7c4f43 to
7f2791c
Compare
|
Does this patch require a newer python version? for these tests |
|
We
We are seeing the same failures in Fuchsia toolchain builders as well. |
|
Thanks for your report. No that is not intended see. Python 3.8 seems not to be tested in any libc++/LLVM so the error didn't trigger earlier. I'll commit a fix shortly, please let me know when the issue persists after the fix. |
mordante
added a commit
that referenced
this pull request
Feb 11, 2025
This addresses the post-commit issues reported in #101880.
Icohedron
pushed a commit
to Icohedron/llvm-project
that referenced
this pull request
Feb 11, 2025
This changes the code to use dataclasses instead of dict entries. It also adds type aliases to use in the typing information and updates the typing information.
Icohedron
pushed a commit
to Icohedron/llvm-project
that referenced
this pull request
Feb 11, 2025
This addresses the post-commit issues reported in llvm#101880.
|
Thanks for the fix @mordante ! |
flovent
pushed a commit
to flovent/llvm-project
that referenced
this pull request
Feb 13, 2025
This addresses the post-commit issues reported in llvm#101880.
joaosaffran
pushed a commit
to joaosaffran/llvm-project
that referenced
this pull request
Feb 14, 2025
This addresses the post-commit issues reported in llvm#101880.
sivan-shani
pushed a commit
to sivan-shani/llvm-project
that referenced
this pull request
Feb 24, 2025
This changes the code to use dataclasses instead of dict entries. It also adds type aliases to use in the typing information and updates the typing information.
sivan-shani
pushed a commit
to sivan-shani/llvm-project
that referenced
this pull request
Feb 24, 2025
This addresses the post-commit issues reported in llvm#101880.
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.
This changes the code to use dataclasses instead of dict entries. It also adds type aliases to use in the typing information and updates the typing information.