[libc][hdrgen] Extend guard attribute support for types#191663
Open
[libc][hdrgen] Extend guard attribute support for types#191663
Conversation
Co-authored-by: un-pixelated <masterhc321@gmail.com>
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
Member
|
@llvm/pr-subscribers-libc Author: Mohammed Ashraf (Holo-xy) ChangesCloses #187404 Full diff: https://github.com/llvm/llvm-project/pull/191663.diff 8 Files Affected:
diff --git a/libc/utils/hdrgen/hdrgen/header.py b/libc/utils/hdrgen/hdrgen/header.py
index e2ebbdb6e519a..fb97e1129cb1a 100644
--- a/libc/utils/hdrgen/hdrgen/header.py
+++ b/libc/utils/hdrgen/hdrgen/header.py
@@ -175,6 +175,7 @@ def includes(self):
PurePosixPath("llvm-libc-types") / f"{typ.name}.h",
)
for typ in self.all_types()
+ if typ.guard is None
}
| {
PurePosixPath("llvm-libc-macros") / f"{attr.split('(')[0]}.h"
@@ -239,7 +240,7 @@ def relpath(file):
# It's implicitly emitted here when using the default template so
# it can get the right relative path. Custom template files should
# all have it explicitly with their right particular relative path.
- return [
+ content = [
f"#include {file}"
for file in ([f'"{relpath(COMMON_HEADER)!s}"'] if with_common else [])
+ sorted(
@@ -248,6 +249,25 @@ def relpath(file):
)
]
+ # Add guarded types
+ current_guard = None
+ has_seen_guard = False
+ for typ in sorted(self.types):
+ if typ.guard is None:
+ continue
+ if not has_seen_guard:
+ has_seen_guard = True
+ content.append("")
+ path = COMPILER_HEADER_TYPES.get(
+ typ.name,
+ PurePosixPath("llvm-libc-types") / f"{typ.name}.h",
+ )
+ current_guard = self.__emit_guard(content, current_guard, typ.guard)
+ content.append(f'#include "{relpath(path)!s}"')
+ self.__emit_guard(content, current_guard, None)
+
+ return content
+
def macro_lines(self):
content = []
for macro in sorted(self.macros):
@@ -295,32 +315,13 @@ def public_api(self):
# elide the blank line between the declarations.
if last_name == function.name_without_underscores():
content.pop()
- if function.guard == None and current_guard == None:
- content.append(str(function) + " __NOEXCEPT;")
- content.append("")
- else:
- if current_guard == None:
- current_guard = function.guard
- content.append(f"#ifdef {current_guard}")
- content.append(str(function) + " __NOEXCEPT;")
- content.append("")
- elif current_guard == function.guard:
- content.append(str(function) + " __NOEXCEPT;")
- content.append("")
- else:
- content.pop()
- content.append(f"#endif // {current_guard}")
- content.append("")
- current_guard = function.guard
- if current_guard is not None:
- content.append(f"#ifdef {current_guard}")
- content.append(str(function) + " __NOEXCEPT;")
- content.append("")
- last_name = function.name_without_underscores()
- if current_guard != None:
- content.pop()
- content.append(f"#endif // {current_guard}")
+ current_guard = self.__emit_guard(
+ content, current_guard, function.guard
+ )
+ content.append(str(function) + " __NOEXCEPT;")
content.append("")
+ last_name = function.name_without_underscores()
+ self.__emit_guard(content, current_guard, None)
# Emit object declarations.
content.extend(str(object) for object in self.objects)
@@ -338,3 +339,14 @@ def json_data(self):
"standards": self.standards,
"includes": sorted(str(file) for file in {COMMON_HEADER} | self.includes()),
}
+
+ def __emit_guard(self, content, current_guard, new_guard):
+ if current_guard != new_guard:
+ if current_guard is not None:
+ if content[-1] == "":
+ content.pop()
+ content.append(f"#endif // {current_guard}")
+ content.append("")
+ if new_guard is not None:
+ content.append(f"#ifdef {new_guard}")
+ return new_guard
diff --git a/libc/utils/hdrgen/hdrgen/type.py b/libc/utils/hdrgen/hdrgen/type.py
index 20c1881a9379a..3a1f3e2eefe93 100644
--- a/libc/utils/hdrgen/hdrgen/type.py
+++ b/libc/utils/hdrgen/hdrgen/type.py
@@ -10,6 +10,7 @@
class Type(Symbol):
- # A type so far carries no specific information beyond its name.
- def __init__(self, name):
+ # A type carries its name and an optional guard.
+ def __init__(self, name, guard=None):
super().__init__(name)
+ self.guard = guard
diff --git a/libc/utils/hdrgen/hdrgen/yaml_to_classes.py b/libc/utils/hdrgen/hdrgen/yaml_to_classes.py
index 85aa3267b5274..13f75dab60511 100644
--- a/libc/utils/hdrgen/hdrgen/yaml_to_classes.py
+++ b/libc/utils/hdrgen/hdrgen/yaml_to_classes.py
@@ -52,7 +52,21 @@ def yaml_to_classes(yaml_data, header_class, entry_points=None):
types = yaml_data.get("types", [])
sorted_types = sorted(types, key=lambda x: x["type_name"])
for type_data in sorted_types:
- header.add_type(Type(type_data["type_name"]))
+ type_name = type_data["type_name"]
+ type_guard = type_data.get("guard")
+ # If a type has a guard, the macro it references must exist in
+ # the same yaml file with a macro_header attribute.
+ if type_guard is not None and not any(
+ macro_data["macro_name"] == type_guard
+ and "macro_header" in macro_data
+ for macro_data in yaml_data.get("macros", [])
+ ):
+ raise ValueError(
+ f"Type '{type_name}' has guard '{type_guard}' but no macro with "
+ f"macro_header '{type_guard}' was found in this file."
+ )
+
+ header.add_type(Type(type_name, type_guard))
for enum_data in yaml_data.get("enums", []):
header.add_enumeration(
diff --git a/libc/utils/hdrgen/tests/expected_output/func_guarding.h b/libc/utils/hdrgen/tests/expected_output/func_guarding.h
new file mode 100644
index 0000000000000..440710e691d4b
--- /dev/null
+++ b/libc/utils/hdrgen/tests/expected_output/func_guarding.h
@@ -0,0 +1,34 @@
+//===-- Standard C header <func_guarding.h> --===//
+//
+// 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
+//
+//===---------------------------------------------------------------------===//
+
+#ifndef _LLVM_LIBC_FUNC_GUARDING_H
+#define _LLVM_LIBC_FUNC_GUARDING_H
+
+#include "__llvm-libc-common.h"
+
+__BEGIN_C_DECLS
+
+#ifdef LIBC_TYPES_HAS_FLOAT128
+void func_all_guarded(int) __NOEXCEPT;
+#endif // LIBC_TYPES_HAS_FLOAT128
+
+#ifdef LIBC_TYPES_HAS_FLOAT16
+int func_guarded_a(int) __NOEXCEPT;
+
+int func_guarded_b(int) __NOEXCEPT;
+#endif // LIBC_TYPES_HAS_FLOAT16
+
+#ifdef LIBC_TYPES_HAS_FLOAT128
+int func_guarded_c(int) __NOEXCEPT;
+#endif // LIBC_TYPES_HAS_FLOAT128
+
+int func_plain(int) __NOEXCEPT;
+
+__END_C_DECLS
+
+#endif // _LLVM_LIBC_FUNC_GUARDING_H
diff --git a/libc/utils/hdrgen/tests/expected_output/type_guarding.h b/libc/utils/hdrgen/tests/expected_output/type_guarding.h
new file mode 100644
index 0000000000000..33a6fbd22d8d0
--- /dev/null
+++ b/libc/utils/hdrgen/tests/expected_output/type_guarding.h
@@ -0,0 +1,27 @@
+//===-- Standard C header <type_guarding_mixed.h> --===//
+//
+// 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
+//
+//===---------------------------------------------------------------------===//
+
+#ifndef _LLVM_LIBC_TYPE_GUARDING_MIXED_H
+#define _LLVM_LIBC_TYPE_GUARDING_MIXED_H
+
+#include "__llvm-libc-common.h"
+#include "llvm-libc-macros/float16-macro.h"
+#include "llvm-libc-macros/size_t-macro.h"
+#include "llvm-libc-types/myType.h"
+#include <stdint.h>
+
+#ifdef LIBC_TYPES_HAS_FLOAT16
+#include "llvm-libc-types/float16.h"
+#endif // LIBC_TYPES_HAS_FLOAT16
+
+#ifdef LIBC_TYPES_HAS_SIZE_T
+#include "llvm-libc-types/size_t.h"
+#include "llvm-libc-types/ssize_t.h"
+#endif // LIBC_TYPES_HAS_SIZE_T
+
+#endif // _LLVM_LIBC_TYPE_GUARDING_MIXED_H
diff --git a/libc/utils/hdrgen/tests/input/func_guarding.yaml b/libc/utils/hdrgen/tests/input/func_guarding.yaml
new file mode 100644
index 0000000000000..eb4093d235e17
--- /dev/null
+++ b/libc/utils/hdrgen/tests/input/func_guarding.yaml
@@ -0,0 +1,38 @@
+header: func_guarding.h
+standards:
+ - stdc
+functions:
+ - name: func_plain
+ return_type: int
+ arguments:
+ - type: int
+ standards:
+ - stdc
+ - name: func_guarded_a
+ return_type: int
+ arguments:
+ - type: int
+ standards:
+ - stdc
+ guard: LIBC_TYPES_HAS_FLOAT16
+ - name: func_guarded_b
+ return_type: int
+ arguments:
+ - type: int
+ standards:
+ - stdc
+ guard: LIBC_TYPES_HAS_FLOAT16
+ - name: func_guarded_c
+ return_type: int
+ arguments:
+ - type: int
+ standards:
+ - stdc
+ guard: LIBC_TYPES_HAS_FLOAT128
+ - name: func_all_guarded
+ return_type: void
+ arguments:
+ - type: int
+ standards:
+ - stdc
+ guard: LIBC_TYPES_HAS_FLOAT128
diff --git a/libc/utils/hdrgen/tests/input/type_guarding.yaml b/libc/utils/hdrgen/tests/input/type_guarding.yaml
new file mode 100644
index 0000000000000..b1e7fb14e5d3b
--- /dev/null
+++ b/libc/utils/hdrgen/tests/input/type_guarding.yaml
@@ -0,0 +1,19 @@
+header: type_guarding_mixed.h
+standards:
+ - stdc
+
+macros:
+ - macro_name: LIBC_TYPES_HAS_SIZE_T
+ macro_header: size_t-macro.h
+ - macro_name: LIBC_TYPES_HAS_FLOAT16
+ macro_header: float16-macro.h
+
+types:
+ - type_name: size_t
+ guard: LIBC_TYPES_HAS_SIZE_T
+ - type_name: ssize_t
+ guard: LIBC_TYPES_HAS_SIZE_T
+ - type_name: float16
+ guard: LIBC_TYPES_HAS_FLOAT16
+ - type_name: uint8_t
+ - type_name: myType
diff --git a/libc/utils/hdrgen/tests/test_integration.py b/libc/utils/hdrgen/tests/test_integration.py
index 8848afe29d50f..ef7e158952cfc 100644
--- a/libc/utils/hdrgen/tests/test_integration.py
+++ b/libc/utils/hdrgen/tests/test_integration.py
@@ -96,6 +96,20 @@ def test_generate_macro_only_header(self):
self.run_script(yaml_file, output_file)
self.compare_files(output_file, expected_output_file)
+ def test_type_guarding(self):
+ yaml_file = self.source_dir / "input/type_guarding.yaml"
+ expected_output_file = self.source_dir / "expected_output/type_guarding.h"
+ output_file = self.output_dir / "type_guarding.h"
+ self.run_script(yaml_file, output_file)
+ self.compare_files(output_file, expected_output_file)
+
+ def test_func_guarding(self):
+ yaml_file = self.source_dir / "input/func_guarding.yaml"
+ expected_output_file = self.source_dir / "expected_output/func_guarding.h"
+ output_file = self.output_dir / "func_guarding.h"
+ self.run_script(yaml_file, output_file)
+ self.compare_files(output_file, expected_output_file)
+
def main():
parser = argparse.ArgumentParser(description="TestHeaderGenIntegration arguments")
parser.add_argument(
|
bassiounix
requested review from
bassiounix,
frobtech,
michaelrj-google and
vhscampos
|
✅ With the latest revision this PR passed the Python code formatter. |
Contributor
|
@Holo-xy |
vhscampos
requested changes
Apr 14, 2026
vhscampos
reviewed
Apr 14, 2026
| "includes": sorted(str(file) for file in {COMMON_HEADER} | self.includes()), | ||
| } | ||
|
|
||
| def __emit_guard(self, content, current_guard, new_guard): |
Member
There was a problem hiding this comment.
This function naming, with leading underscores, is not used anywhere else in the file. I don't think it's a good idea to introduce it only for this new function
Member
There was a problem hiding this comment.
Are you talking about the naming style?
IIRC this is how you make private methods in python (using double underscores).
Member
There was a problem hiding this comment.
Yes.
But AFAIK, that sort of naming style is just a convention as Python doesn't have enforceable function visibility in that sense. In any case, pretty much all the functions in this file are "private" as well and they don't have leading underscores. We should choose only one style and make the whole file follow it.
vhscampos
reviewed
Apr 14, 2026
| content.append("") | ||
| if new_guard is not None: | ||
| content.append(f"#ifdef {new_guard}") | ||
| return new_guard |
Member
There was a problem hiding this comment.
The function can only ever return one value, and this value is from one of its inputs. So there's little sense in doing it this way.
I suggest you simply don't return anything from this function, and just use the argument value at the call sites directly.
vhscampos
reviewed
Apr 14, 2026
| if current_guard != None: | ||
| content.pop() | ||
| content.append(f"#endif // {current_guard}") | ||
| current_guard = self.__emit_guard(content, current_guard, function.guard) |
Member
There was a problem hiding this comment.
Based on my other comment, this would look like this:
self.emit_guard(content, current_guard, function.guard)
current_guard = function.guard
vhscampos
requested changes
Apr 14, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
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.
Closes #187404