[flang-commits] [PATCH] D80377: [flang] Google test infrastructure support for unittests

[Tim Keith via Phabricator via flang-commits]

tskeith added inline comments.


================
Comment at: flang/CMakeLists.txt:128
+  set(UNITTEST_DIR ${LLVM_BUILD_MAIN_SRC_DIR}/utils/unittest)
+  if(EXISTS ${UNITTEST_DIR}/googletest/include/gtest/gtest.h)
+    if (TARGET gtest)
----------------
Why do you have to check for `gtest.h`? Won't it always be there?


================
Comment at: flang/CMakeLists.txt:129
+  if(EXISTS ${UNITTEST_DIR}/googletest/include/gtest/gtest.h)
+    if (TARGET gtest)
+      # LLVM Doesn't export gtest's include directorys, so do that here
----------------
Same question. Why do you have to check this?


================
Comment at: flang/include/flang/Optimizer/Support/InternalNames.h:68
   std::string doConstant(llvm::ArrayRef<llvm::StringRef> modules,
+		         llvm::Optional<llvm::StringRef> host,
                          llvm::StringRef name);
----------------
This doesn't look properly formatted. Is it  part of your change?


================
Comment at: flang/runtime/io-error.cpp:10
 #include "io-error.h"
-#include "config.h"
 #include "magic-numbers.h"
----------------
Is this supposed to be part of your change?


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

https://reviews.llvm.org/D80377