Try to address comments in PR

Author: mcbarton

CMakeLists.txt

@@ -366,7 +366,7 @@ if (LLVM_COMPILER_IS_GCC_COMPATIBLE)
   else()
     set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fno-common -Woverloaded-virtual -Wcast-qual -fno-strict-aliasing -pedantic -Wno-long-long -Wall -W -Wno-unused-parameter -Wwrite-strings")
   endif ()
-  # Needed due an error which occurs when you compile gtest on emscripten
+  # Needed due to an error which occurs when you compile gtest on emscripten
   if (EMSCRIPTEN)
     set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}  -Wno-sign-compare")
   endif()

cmake/modules/GoogleTest.cmake

@@ -13,62 +13,38 @@ if(WIN32)
     -DCMAKE_ARCHIVE_OUTPUT_DIRECTORY_MINSIZEREL:PATH=${_gtest_byproduct_binary_dir}/lib/
     -DCMAKE_ARCHIVE_OUTPUT_DIRECTORY_RELEASE:PATH=${_gtest_byproduct_binary_dir}/lib/
     -DCMAKE_ARCHIVE_OUTPUT_DIRECTORY_RELWITHDEBINFO:PATH=${_gtest_byproduct_binary_dir}/lib/
-    -Dgtest_force_shared_crt=ON
-    BUILD_COMMAND ${CMAKE_COMMAND} --build <BINARY_DIR> --config $<CONFIG>)
+    -Dgtest_force_shared_crt=ON)
 elseif(APPLE)
   set(EXTRA_GTEST_OPTS -DCMAKE_OSX_SYSROOT=${CMAKE_OSX_SYSROOT})
 endif()
 
 include(ExternalProject)
-if(EMSCRIPTEN)
-
-ExternalProject_Add(
-  googletest
-  GIT_REPOSITORY https://github.com/google/googletest.git
-  GIT_SHALLOW 1
-  GIT_TAG v1.15.2
-  UPDATE_COMMAND ""
-  # # Force separate output paths for debug and release builds to allow easy
-  # # identification of correct lib in subsequent TARGET_LINK_LIBRARIES commands
-  # CMAKE_ARGS -DCMAKE_ARCHIVE_OUTPUT_DIRECTORY_DEBUG:PATH=DebugLibs
-  #            -DCMAKE_ARCHIVE_OUTPUT_DIRECTORY_RELEASE:PATH=ReleaseLibs
-  #            -Dgtest_force_shared_crt=ON
-  CONFIGURE_COMMAND emcmake cmake -DCMAKE_CXX_FLAGS=${CMAKE_CXX_FLAGS}
+if (EMSCRIPTEN)
+  set(CONFIGURE_COMMAND emcmake cmake -DCMAKE_CXX_FLAGS=${CMAKE_CXX_FLAGS}
   		    -S ${CMAKE_BINARY_DIR}/unittests/googletest-prefix/src/googletest/
-		    -B ${CMAKE_BINARY_DIR}/unittests/googletest-prefix/src/googletest-build/
-  BUILD_COMMAND emmake make
-  # Disable install step
-  INSTALL_COMMAND ""
-  BUILD_BYPRODUCTS ${_gtest_byproducts}
-  # Wrap download, configure and build steps in a script to log output
-  LOG_DOWNLOAD ON
-  LOG_CONFIGURE ON
-  LOG_BUILD ON
-  TIMEOUT 600
-  )
-
+		    -B ${CMAKE_BINARY_DIR}/unittests/googletest-prefix/src/googletest-build/)
+  set(BUILD_COMMAND emmake make)
 else()
-
-  ExternalProject_Add(
-    googletest
-    GIT_REPOSITORY https://github.com/google/googletest.git
-    GIT_SHALLOW 1
-    GIT_TAG v1.15.2
-    UPDATE_COMMAND ""
-    # # Force separate output paths for debug and release builds to allow easy
-    # # identification of correct lib in subsequent TARGET_LINK_LIBRARIES commands
-    # CMAKE_ARGS -DCMAKE_ARCHIVE_OUTPUT_DIRECTORY_DEBUG:PATH=DebugLibs
-    #            -DCMAKE_ARCHIVE_OUTPUT_DIRECTORY_RELEASE:PATH=ReleaseLibs
-    #            -Dgtest_force_shared_crt=ON
-    CMAKE_ARGS -G ${CMAKE_GENERATOR}
+  set(CONFIGURE_COMMAND CMAKE_ARGS -G ${CMAKE_GENERATOR}
                 -DCMAKE_BUILD_TYPE=$<CONFIG>
                 -DCMAKE_C_COMPILER=${CMAKE_C_COMPILER}
                 -DCMAKE_C_FLAGS=${CMAKE_C_FLAGS}
                 -DCMAKE_CXX_COMPILER=${CMAKE_CXX_COMPILER}
                 -DCMAKE_CXX_FLAGS=${CMAKE_CXX_FLAGS}
                 -DCMAKE_AR=${CMAKE_AR}
                 -DCMAKE_INSTALL_PREFIX=${CMAKE_INSTALL_PREFIX}
-                ${EXTRA_GTEST_OPTS}
+                ${EXTRA_GTEST_OPTS})
+  set(BUILD_COMMAND ${CMAKE_COMMAND} --build <BINARY_DIR> --config $<CONFIG>)
+endif()
+
+  ExternalProject_Add(
+    googletest
+    GIT_REPOSITORY https://github.com/google/googletest.git
+    GIT_SHALLOW 1
+    GIT_TAG v1.15.2
+    UPDATE_COMMAND ""
+    CONFIGURE_COMAND ${CONFIGURE_COMMAND}
+    BUILD_COMMMAND ${BUILD_COMMAND}
     # Disable install step
     INSTALL_COMMAND ""
     BUILD_BYPRODUCTS ${_gtest_byproducts}

unittests/CppInterOp/CMakeLists.txt

@@ -1,8 +1,16 @@
 set(LLVM_LINK_COMPONENTS
   Support
 )
+if (EMSCRIPTEN)
+  # 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 explictly main.cpp is needed
+  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
+  set(EXTRA_TEST_SOURCE_FILES CUDATest.cpp)
+endif()
 
-if(EMSCRIPTEN)
 add_cppinterop_unittest(CppInterOpTests
   EnumReflectionTest.cpp
   FunctionReflectionTest.cpp
@@ -12,21 +20,8 @@ add_cppinterop_unittest(CppInterOpTests
   TypeReflectionTest.cpp
   Utils.cpp
   VariableReflectionTest.cpp
-  main.cpp
-)
-else()
-  add_cppinterop_unittest(CppInterOpTests
-  CUDATest.cpp
-  EnumReflectionTest.cpp
-  FunctionReflectionTest.cpp
-  InterpreterTest.cpp
-  JitTest.cpp
-  ScopeReflectionTest.cpp
-  TypeReflectionTest.cpp
-  Utils.cpp
-  VariableReflectionTest.cpp
+  ${EXTRA_TEST_SOURCE_FILES}
 )
-endif()
 
 
 
@@ -67,7 +62,7 @@ export_executable_symbols(CppInterOpTests)
 unset(LLVM_LINK_COMPONENTS)
 
 if(EMSCRIPTEN)
-  add_cppinterop_unittest(DynamicLibraryManagerTests DynamicLibraryManagerTest.cpp main.cpp)
+  add_cppinterop_unittest(DynamicLibraryManagerTests DynamicLibraryManagerTest.cpp ${EXTRA_TEST_SOURCE_FILES})
 else()
   add_cppinterop_unittest(DynamicLibraryManagerTests DynamicLibraryManagerTest.cpp)
 endif()