[libc][docs] Fix docgen macro lookup for underscored headers#194367
Conversation
|
@llvm/pr-subscribers-libc Author: Petter Berntsson (petbernt) ChangesWhile adding implementation status for nl_types.h, I noticed docgen resolves it to nl-types.h instead of nl_types.h. As a result, headers with underscores are not matched correctly and their implementation status is not marked. This patch fixes the handling of underscored header names in docgen so they are processed consistently. Full diff: https://github.com/llvm/llvm-project/pull/194367.diff 1 Files Affected:
diff --git a/libc/utils/docgen/header.py b/libc/utils/docgen/header.py
index 8885db42f0a5b..d43831eb48af8 100644
--- a/libc/utils/docgen/header.py
+++ b/libc/utils/docgen/header.py
@@ -112,5 +112,7 @@ def __get_macro_files(self) -> Generator[Path, None, None]:
instead use a hyphen in the name.
libc/include/llvm-libc-macros/sys-mman-macros.h
"""
- stem = self.stem.replace("/", "-")
+ # Some POSIX headers use underscores in the header name but hyphens in
+ # the macro file name, e.g. nl_types.h -> nl-types-macros.h.
+ stem = self.stem.replace("/", "-").replace("_", "-")
return self.macros_dir.glob(f"**/{stem}-macros.h")
|
| stem = self.stem.replace("/", "-") | ||
| # Some POSIX headers use underscores in the header name but hyphens in | ||
| # the macro file name, e.g. nl_types.h -> nl-types-macros.h. | ||
| stem = self.stem.replace("/", "-").replace("_", "-") |
There was a problem hiding this comment.
Maybe unrelated to the change, but wouldn't it be better if we renamed nl-types-macros.h to nl_types-macros.h?
There was a problem hiding this comment.
I think either approach works, but renaming nl-types-macros.h to nl_types-macros.h is probably the cleaner fix since it removes the naming inconsistency at the source.
This patch was only meant to make the implementation-status lookup match the current repository state.
I’m happy to switch to the rename instead!
There was a problem hiding this comment.
@michaelrj-google Do you have a preference here? I'm inclined to say that the filenames are probably wrong and should be fixed, but I want to make sure you don't have something in mind here.
There was a problem hiding this comment.
I went ahead and switched this to the rename-based fix since that seemed like the cleaner direction.
Happy to adjust if there’s a preference for the original docgen-based approach.
There was a problem hiding this comment.
in general I prefer the simplest solution. As a short term fix I think the PR only renaming nl_types is the best solution, then a followup PR to move all the relevant POSIX headers along with the docgen change would make sense.
petbernt
force-pushed
the
libc-docs-fix-docgen-underscored-headers
branch
from
a1e12c0 to
e33f7d0
Compare
|
Feel free to merge it for me as well, as I do not have merge permissions. |
…lvm#194373) Add nl_types.h implementation-status docs to llvm-libc. Depends on PR llvm#194367. That change fixes docgen lookup for underscored headers, without it, the macros of nl_types.h implementation status is not reported accurately.
…4367) While adding implementation status for nl_types.h, I noticed docgen resolves it to nl-types.h instead of nl_types.h. As a result, headers with underscores are not matched correctly and their implementation status is not marked. This patch fixes the handling of underscored header names in docgen so they are processed consistently.
…lvm#194373) Add nl_types.h implementation-status docs to llvm-libc. Depends on PR llvm#194367. That change fixes docgen lookup for underscored headers, without it, the macros of nl_types.h implementation status is not reported accurately.
…4367) While adding implementation status for nl_types.h, I noticed docgen resolves it to nl-types.h instead of nl_types.h. As a result, headers with underscores are not matched correctly and their implementation status is not marked. This patch fixes the handling of underscored header names in docgen so they are processed consistently.
5 participants
While adding implementation status for nl_types.h, I noticed docgen resolves it to nl-types.h instead of nl_types.h. As a result, headers with underscores are not matched correctly and their implementation status is not marked.
This patch fixes the handling of underscored header names in docgen so they are processed consistently.