[libc-commits] [PATCH] D128908: [libc][utils] Add more methods to StringView

[Siva Chandra via Phabricator via libc-commits]

sivachandra added inline comments.


================
Comment at: libc/src/__support/CPP/StringView.h:22
 // This class will be extended as needed in future.
 class StringView {
 private:
----------------
courbet wrote:
> @sivachandra Do we really want to keep duplicating the whole libc++ in there ? I can see why we can't use gtest to test the libc, but we should be able to use gtest to test individual internal components. Then we would not need to reimplement the world.
> 
> internal unit tests would test `__llvm_libc::CodeUnderTest` using the system libc and `gtest`
> libc API tests would use the extern C `CodeUnderTest` build with `-ffrestanding` and using `llvm-libc` and `LibCTest`
> 
> Does that make sense ? At least for code that doe snot rely on `errno` and such weird things.
> @sivachandra Do we really want to keep duplicating the whole libc++ in there ?

Just pointing out, a large part of that duplicated code is used in the actual libc runtime code. So, while it might seem like unnecessarily duplicated especially when it seems like it is being used only for testing, it is not going to waste.

> I can see why we can't use gtest to test the libc, but we should be able to use gtest to test individual internal components. Then we would not need to reimplement the world.
> 
> internal unit tests would test `__llvm_libc::CodeUnderTest` using the system libc and `gtest`
> libc API tests would use the extern C `CodeUnderTest` build with `-ffrestanding` and using `llvm-libc` and `LibCTest`

We almost never test the C symbols in our unittests. All of our unittests test members in the `__llvm_libc` namespace. So, if we go with your suggestion, then we can stick with gtest almost always. But, we have moved away from using gtest because we build and run our unittests in two different modes:

    1. The default mode in which we use public headers from the system libc. Of course, only those functions which do not depend on the definitions in the header files can be tested in this manner. For example, most of the functions in `string.h` and `math.h`. The unittests for those functions can very well use gtest, include the C++ standard library etc and it should all just work fine.
    2. The //full build// mode in which we test the libc against its own public headers. This mode prevents us from including the C++ standard library headers, gtest headers or any other third party library headers as they can themselves potentially include the libc headers. 

> Does that make sense ? At least for code that doe snot rely on `errno` and such weird things.

Beyond `errno`, not so weird things like macro definitions and type definitions can also cause problems in the full build mode.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128908