[libc-commits] [PATCH] D84893: [libc] Implements isdigit and isalnum.

Chris Gyurgyik via Phabricator via libc-commits libc-commits at lists.llvm.org
Thu Jul 30 04:52:13 PDT 2020


cgyurgyik added a comment.

In D84893#2183796 <https://reviews.llvm.org/D84893#2183796>, @sivachandra wrote:

> Overall LGTM. To avoid call instructions to other classification functions, for example `isalnum` calls `isdigit` and or `isalpha` does it make sense to have a library of `static inline` implementations in a header file. Then, the implementation functions will call the `static inline` functions, but the call overhead is avoided because of them being `static inline`. WDYT?

Inlining is good, but comes at a price of abstraction.

1. In this case, we probably want to use the static inline definition as the one provided for `isdigit` as well, right? Even though, its a single line of code, I think its better to avoid defining it twice.
2. I'm assuming we want to do this solely for ctype functions that are used in other functions, correct? It might be confusing initially for someone looking at the library, but I don't see a point in having an inlined function that's not called elsewhere.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84893



More information about the libc-commits mailing list