[libc] Make more of the libc unit testing llvm independent

[michaelrj]

Summary

This removes the rest of the llvm dependencies from the libc unit test framework.

[sivachandra]

libc/utils/UnitTest/CMakeLists.txt

5

You should probably do the same in utils/testutils/CMakeLists.txt? I will do another pass and switch these targets to add_library after I move FPUtil's TestHelpers.

libc/utils/testutils/ExecuteFunctionUnix.cpp

12

Why do we need cstring?

[michaelrj]

(NOT QUITE WORKING) get the rest of the llvm dependencies out StreamWrapper and FDReader

my FDReader change isn't quite working yet but I need to go work on something else and I
don't want to forget to upload this

[michaelrj]

submit comments

libc/utils/UnitTest/CMakeLists.txt

5

I'll do that once I get the rest of the llvm dependencies out of utils/testutils which should be pretty soon after I get this next commit working.

libc/utils/testutils/ExecuteFunctionUnix.cpp

12

because otherwise strsignal depends on it (line 81)

[abrachet]

libc/utils/testutils/FDReaderUnix.cpp

20–23 ↗

(On Diff #306222)

Needs brackets and no comment

32–35 ↗

(On Diff #306222)

const -> constexpr
I don't think you need to justify the chunk size with the comment.

40 ↗

(On Diff #306222)

Probably left over from testing.

42–43 ↗

(On Diff #306222)

This loop could maybe be
for (ssize_t bytesRead; (bytesRead = ::read(...)); ) And then remove the bytesRead == 0 check

45 ↗

(On Diff #306222)

Read will not null terminate the buffer, the input probably won't have a '\0' in it either. I think pipeStr.insert(pipeStr.end(), buffer, bytesRead) or similar is safer.

54 ↗

(On Diff #306222)

I think pipeStr == inputStr is more clear
And remove the comment below

[michaelrj]

This change finishes the work started in the previous one, all of the tests now pass.
Big thank you to abrachet for the comments.

[michaelrj]

submit comments

libc/utils/testutils/FDReaderUnix.cpp

20–23 ↗

(On Diff #306222)

I feel very silly for missing that one. That was why the test was failing immediately.

42–43 ↗

(On Diff #306222)

I made one small change to your code, bytesRead needs to be signed because read uses negative numbers for errors

54 ↗

(On Diff #306222)

oops, as I'm sure you guessed I forgot to clean up my code before submitting.

[sivachandra]

I am not an expert with pipes. To my eyes, looks OK.

libc/utils/testutils/FDReaderUnix.cpp

10 ↗

(On Diff #306525)

Remove this?

[michaelrj]

remove commented out include

libc/utils/UnitTest/CMakeLists.txt

 add_llvm_library(
   LibcUnitTest
   Test.cpp
   Test.h
-  LINK_COMPONENTS Support
 )
 target_include_directories(LibcUnitTest PUBLIC ${LIBC_SOURCE_DIR})
 add_dependencies(LibcUnitTest libc.utils.CPP.standalone_cpp)
 target_link_libraries(LibcUnitTest PUBLIC libc_test_utils)

libc/utils/testutils/ExecuteFunctionUnix.cpp

 //===-- ExecuteFunction implementation for Unix-like Systems --------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "ExecuteFunction.h"
-#include "llvm/Support/raw_ostream.h"
 #include <cassert>
 #include <cstdlib>
+#include <cstring>
+#include <iostream>
 #include <memory>
 #include <poll.h>
 #include <signal.h>
 #include <sys/wait.h>
 #include <unistd.h>
 
 namespace __llvm_libc {
 namespace testutils {

 
 ProcessStatus invokeInSubprocess(FunctionCaller *Func, unsigned timeoutMS) {
   std::unique_ptr<FunctionCaller> X(Func);
   int pipeFDs[2];
   if (::pipe(pipeFDs) == -1)
     return ProcessStatus::Error("pipe(2) failed");
 
   // Don't copy the buffers into the child process and print twice.
-  llvm::outs().flush();
-  llvm::errs().flush();
+  std::cout.flush();
+  std::cerr.flush();
   pid_t Pid = ::fork();
   if (Pid == -1)
     return ProcessStatus::Error("fork(2) failed");
 
   if (!Pid) {
     (*Func)();
     std::exit(0);
   }