[libc-commits] [PATCH] D89267: Move Cannonical Function List

[Michael Jones via Phabricator via libc-commits]

michaelrj added inline comments.


================
Comment at: libc/utils/HdrGen/PublicAPICommand.cpp:92
     if (G.FunctionSpecMap.find(Name) == G.FunctionSpecMap.end())
-      llvm::PrintFatalError(Name + " not found in any standard spec.\n");
+      continue; // this used to be an error, but isn't anymore because
+                // we're getting the full function list from entrypoints.txt
----------------
sivachandra wrote:
> sivachandra wrote:
> > michaelrj wrote:
> > > sivachandra wrote:
> > > > When running the `PublicAPICommand`, the `FunctionSpecMap` will be for the header file we are generating. So, it should still be an error if a function is not found, no?
> > > The `FunctionSpecMap` is indeed for the header file we're generating, but that line is checking if each function in the list `Functions` is in `FunctionSpecMap`. Previously the list in `Functions` (or `G.Functions`) was supposed to be identical to that list, but since we're now ignoring what header file we're looking for in `getFunctionsFromEntrypointsFile`, the list `Functions` contains every function listed in `entrypoints.txt`. When I run the  code with this as an error it crashes when trying to load a header file because not all functions listed are in that header file.
> > Ah, got it. Thanks for explaining. Instead of a dangling comment, can you format like this:
> > 
> > ```
> > if (...) {
> >   // comment
> >   // comment
> >   // ...
> >   continue;
> > }
> > ```
> > 
> > Also, try to use correct grammar like beginning sentences with **U**pper case words. You don't need to explain previous code (in fact, it will be confusing if you do) but explain the current code.
> I think you have not addressed this?
Addressed in the new version.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89267/new/

https://reviews.llvm.org/D89267