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

Michael Jones via Phabricator via libc-commits libc-commits at lists.llvm.org
Tue Oct 13 15:40:48 PDT 2020


michaelrj added inline comments.


================
Comment at: libc/spec/llvm_libc_ext.td:29
+               ArgSpec<UnsignedType>,
+	       ArgSpec<ConstCharPtr>,]
+
----------------
lntue wrote:
> alignment
since I'm moving the changes in this file to a different commit the fix will be there, but it has been fixed.


================
Comment at: libc/spec/stdc.td:4
+  //all other types are defined in spec.td, but these ones would collide with
+  //the definition of FILE in libc/config/public_api.td, so this one gets to be scoped.
   NamedType FILE = NamedType<"FILE">;
----------------
sivachandra wrote:
> I did not get this comment. Can you explain a bit more?
since I'm moving the changes in this file to a different commit I'll update the comment there.


================
Comment at: libc/utils/HdrGen/EntrypointsLoader.cpp:31
+
+  EntrypointsLoader::NameSet FunctionSet = *(new EntrypointsLoader::NameSet());
+  std::string HeaderNameStripped = std::string(HeaderName.split(".").first);
----------------
sivachandra wrote:
> Why is a `new` required here? Also, ins't it a memory leak? Isn't a simple stack variable sufficient?
Oops, you're right that shouldn't be on the heap. I was for some reason thinking that the set was going to be passed by reference and not by value.


================
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:
> 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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89267



More information about the libc-commits mailing list