[PATCH] D100442: [flang][msvc] Fix compilation of RuntimeGtest tests.

[مهدي شينون (Mehdi Chinoune) via Phabricator via llvm-commits]

MehdiChinoune added inline comments.


================
Comment at: flang/unittests/RuntimeGTest/CMakeLists.txt:21
   FortranRuntime
+  ${llvm_libs}
 )
----------------
awarzynski wrote:
> MehdiChinoune wrote:
> > awarzynski wrote:
> > > MehdiChinoune wrote:
> > > > awarzynski wrote:
> > > > > This will basically add a new dependency for all configurations - how come this didn't fail before on other platforms? Also, there's a Flang buildbot worker that sets `LLVM_LINK_LLVM_DYLIB` to `On`: https://lab.llvm.org/buildbot/#/builders/135 and it's currently :green:. 
> > > > > 
> > > > > Separately, `Support` is added as a dependency for unit tests here: https://github.com/llvm/llvm-project/blob/main/llvm/cmake/modules/AddLLVM.cmake#L1452.
> > > > > 
> > > > > So, is this change really needed?
> > > > I don't know but the compiler doesn't recognize llvm::errs() `CrashHandlerFixture.cpp:19` Unless explicitly linked with `Support` component, I saw the same here https://github.com/llvm/llvm-project/blob/main/flang/unittests/Evaluate/CMakeLists.txt#L6
> > > The tests that you are referring to are not using GTest and use different CMake macros. The tests defined here use the `add_flang_unittest` macro, which calls LLVM's `add_unittest`. That should be adding `Support` as a dependency.
> > > 
> > > * `add_flang_unittest`: https://github.com/llvm/llvm-project/blob/main/flang/unittests/CMakeLists.txt#L4-L6
> > > * `add_unittestt`: https://github.com/llvm/llvm-project/blob/main/llvm/cmake/modules/AddLLVM.cmake#L1437-L1478
> > > The tests that you are referring to are not using GTest and use different CMake macros. The tests defined here use the `add_flang_unittest` macro, which calls LLVM's `add_unittest`. That should be adding `Support` as a dependency.
> > > 
> > > * `add_flang_unittest`: https://github.com/llvm/llvm-project/blob/main/flang/unittests/CMakeLists.txt#L4-L6
> > > * `add_unittestt`: https://github.com/llvm/llvm-project/blob/main/llvm/cmake/modules/AddLLVM.cmake#L1437-L1478
> > 
> > I don't see any explicit linking to Support.
> I've already linked this above: 
> https://github.com/llvm/llvm-project/blob/main/llvm/cmake/modules/AddLLVM.cmake#L1452
> ```
>   list(APPEND LLVM_LINK_COMPONENTS Support) # gtest needs it for raw_ostream
> ```
> Or am I misreading this?
I have updated the diff.


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

https://reviews.llvm.org/D100442