-
Notifications
You must be signed in to change notification settings - Fork 17.1k
[libc][hdrgen] Extend guard attribute support for types #191663
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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,11 @@ 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 +337,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): | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are you talking about the naming style?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
| 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 | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| //===-- Standard C header <type_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_TYPE_GUARDING_H | ||
| #define _LLVM_LIBC_TYPE_GUARDING_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_H |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| header: type_guarding.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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on my other comment, this would look like this: