[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