[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