[libc-commits] [PATCH] D70197: [libc] Add a TableGen based header generator.
Petr Hosek via Phabricator via libc-commits
libc-commits at lists.llvm.org
Fri Nov 22 00:05:11 PST 2019
phosek accepted this revision.
phosek added inline comments.
================
Comment at: libc/config/linux/api.td:77
+
+def StdIoAPI : PublicAPI<"stdio.h"> {
+ let TypeDeclarations = [
----------------
nit: Should this be spelled as `StdIOAPI` since both `IO` and `API` are acronyms?
================
Comment at: libc/docs/ground_truth_specification.rst:7
+fashion, LLVM libc employs ground truth files. These files live under the
+``spec`` directory and list ground truth corresponding the ISO C standanrd, the
+POSIX extension standard, etc. For example, the path to the ground truth file
----------------
s/standanrd/standard/
================
Comment at: libc/utils/HdrGen/Generator.cpp:38
+ if (CommandName == IncludeFileCommand::Name) {
+ if (!IncludeFileCmd) {
+ IncludeFileCmd = std::make_unique<IncludeFileCommand>();
----------------
LLVM style doesn't use `{` and `}` for blocks with a single statement. There are multiple instances of this throughout this patch.
================
Comment at: libc/utils/HdrGen/Generator.cpp:61
+ A = A.trim(' ');
+ if (A.startswith(ParamNamePrefix) and A.endswith(ParamNameSuffix)) {
+ A = A.drop_front(ParamNamePrefixSize).drop_back(ParamNameSuffixSize);
----------------
Should this use `&&` rather than `and`?
================
Comment at: libc/utils/HdrGen/Generator.cpp:63
+ A = A.drop_front(ParamNamePrefixSize).drop_back(ParamNameSuffixSize);
+ A = ArgMap[A];
+ }
----------------
`A` doesn't seem to be used anywhere after this line?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D70197/new/
https://reviews.llvm.org/D70197
More information about the libc-commits
mailing list