Coverage, resolving comments

Author: kr-2003

.github/workflows/main.yml

@@ -57,8 +57,8 @@ jobs:
             llvm_enable_projects: "clang"
             llvm_targets_to_build: "host;NVPTX"
           # Ubuntu X86 Jobs
-          - name: ubu22-x86-gcc12-clang-repl-20-coverage
-            os: ubuntu-22.04
+          - name: ubu24-x86-gcc12-clang-repl-20-coverage
+            os: ubuntu-24.04
             compiler: gcc-12
             clang-runtime: '20'
             cling: Off
@@ -255,16 +255,14 @@ jobs:
       with:
         cache-hit: ${{ steps.cache.outputs.cache-hit }}
 
-# REVERT BEFORE MERGING
-    # - name: Cache LLVM-${{ matrix.clang-runtime }} and ${{ matrix.cling == 'On' && 'Cling' || 'Clang-REPL' }} build
-    #   uses: actions/cache/save@v4
-    #   if: ${{ steps.cache.outputs.cache-hit != 'true' }}
-    #   with:
-    #     path: |
-    #       llvm-project
-    #       ${{ matrix.cling=='On' && 'cling' || '' }}
-    #     key: ${{ steps.cache.outputs.cache-primary-key }}
-# REVERT BEFORE MERGING
+    - name: Cache LLVM-${{ matrix.clang-runtime }} and ${{ matrix.cling == 'On' && 'Cling' || 'Clang-REPL' }} build
+      uses: actions/cache/save@v4
+      if: ${{ steps.cache.outputs.cache-hit != 'true' }}
+      with:
+        path: |
+          llvm-project
+          ${{ matrix.cling=='On' && 'cling' || '' }}
+        key: ${{ steps.cache.outputs.cache-primary-key }}
 
     - name: Setup code coverage
       if: ${{ success() && (matrix.coverage == true) }}
@@ -282,7 +280,7 @@ jobs:
         # Create lcov report
         # capture coverage info
         vers="${CC#*-}"
-        lcov --directory build/ --capture --output-file coverage.info --gcov-tool /usr/bin/gcov-${vers}
+        lcov --directory build/ --capture --output-file coverage.info --gcov-tool /usr/bin/gcov-${vers} --ignore-errors mismatch
         lcov --remove coverage.info '/usr/*' ${{ github.workspace }}'/llvm-project/*' --ignore-errors unused --output-file coverage.info
         lcov --remove coverage.info '${{ github.workspace }}/unittests/*' --ignore-errors unused --output-file coverage.info
         lcov --remove coverage.info  '${{ github.workspace }}/build/*' --ignore-errors unused --output-file coverage.info

lib/CppInterOp/Compatibility.h

@@ -260,7 +260,7 @@ createClangInterpreter(std::vector<const char*>& args, int stdin_fd = -1,
   if (CudaEnabled)
     DeviceCI->LoadRequestedPlugins();
 
-  bool outOfProcess = (stdin_fd != -1 && stdout_fd != -1 && stderr_fd != -1);
+  bool outOfProcess = (stdin_fd != 0 && stdout_fd != 1 && stderr_fd != 2);
 
 #ifdef LLVM_BUILT_WITH_OOP_JIT
 

lib/CppInterOp/CppInterOpInterpreter.h

@@ -174,9 +174,9 @@ class Interpreter {
   static std::tuple<int, int, int>
   initAndGetFileDescriptors(std::vector<const char*>& vargs,
                             std::unique_ptr<IOContext>& io_ctx) {
-    int stdin_fd = -1;
-    int stdout_fd = -1;
-    int stderr_fd = -1;
+    int stdin_fd = 0;
+    int stdout_fd = 1;
+    int stderr_fd = 2;
     bool outOfProcess = false;
 
 #if defined(_WIN32)
@@ -193,6 +193,9 @@ class Interpreter {
         if (!init) {
           llvm::errs() << "Can't start out-of-process JIT execution.\n";
           outOfProcess = false;
+          stdin_fd = -1;
+          stdout_fd = -1;
+          stderr_fd = -1;
         }
       }
       if (outOfProcess) {
@@ -237,7 +240,12 @@ class Interpreter {
     std::tie(stdin_fd, stdout_fd, stderr_fd) =
         initAndGetFileDescriptors(vargs, io_ctx);
 
-    bool outOfProcess = (stdin_fd != -1 && stdout_fd != -1 && stderr_fd != -1);
+    if (stdin_fd == -1 || stdout_fd == -1 || stderr_fd == -1) {
+      llvm::errs() << "Redirection files creation failed for Out-Of-Process JIT\n";
+      return nullptr;
+    }
+
+    bool outOfProcess = (stdin_fd != 0 && stdout_fd != 1 && stderr_fd != 2);
 
     auto CI =
         compat::createClangInterpreter(vargs, stdin_fd, stdout_fd, stderr_fd);

unittests/CppInterOp/CMakeLists.txt

@@ -1,15 +1,13 @@
-set(EXTRA_TEST_SOURCE_FILES Utils.cpp)
-
 if (EMSCRIPTEN)
   # So we create a html file, as well as the javascript file
   set(CMAKE_EXECUTABLE_SUFFIX ".html")
   # Omitting CUDATest.cpp since Emscripten build currently has no GPU support
   # For Emscripten builds linking to gtest_main will not suffice for gtest to run
   # the tests and an explicitly main.cpp is needed
-  list(APPEND EXTRA_TEST_SOURCE_FILES main.cpp)
+  set(EXTRA_TEST_SOURCE_FILES main.cpp)
 else()
   # Do not need main.cpp for native builds, but we do have GPU support for native builds
-  list(APPEND EXTRA_TEST_SOURCE_FILES CUDATest.cpp)
+  set(EXTRA_TEST_SOURCE_FILES CUDATest.cpp)
   set(EXTRA_PATH_TEST_BINARIES /CppInterOpTests/unittests/bin/$<CONFIG>/)
 endif()
 
@@ -20,6 +18,7 @@ add_cppinterop_unittest(CppInterOpTests
   JitTest.cpp
   ScopeReflectionTest.cpp
   TypeReflectionTest.cpp
+  Utils.cpp
   VariableReflectionTest.cpp
   ${EXTRA_TEST_SOURCE_FILES}
 )

unittests/CppInterOp/InterpreterTest.cpp

@@ -27,13 +27,6 @@
 
 using ::testing::StartsWith;
 
-class InterpreterTest : public ::testing::TestWithParam<TestUtils::TestConfig> {
-protected:
-  void SetUp() override {
-    TestUtils::current_config = GetParam();
-  }
-};
-
 TYPED_TEST(CppInterOpTest, InterpreterTestVersion) {
   EXPECT_THAT(Cpp::GetVersion(), StartsWith("CppInterOp version"));
 }