[PATCH] D93778: [test-suite] Support Fortran tests in CMake infrastructure.

[Michael Kruse via Phabricator via llvm-commits]

Meinersbur added inline comments.


================
Comment at: CMakeLists.txt:31
+if (TEST_SUITE_FORTRAN)
+    mark_as_advanced(CLEAR CMAKE_Fortran_COMPILER)
+endif()
----------------
naromero77 wrote:
> Meinersbur wrote:
> > There is probably a reason why the `_COMPILER` vars are marked as advanced, they cannot be changed after the initial configuration. I don't see a reason to make it non-advanced.
> Was this just a comment or where there any action you wanted me to to take? 
> 
> Here I am just following a parallel constructions similar to the prior `mark_as_advance(...)`.
OK, didn't notice this is also done for CMAKE_C_COMPILER, CMAKE_CXX_COMPILER. I don't this is good, but keeps the Fortran support consistent with C/CXX, like modifying _FLAGS.


================
Comment at: SingleSource/UnitTests/Fortran/CMakeLists.txt:1
+file(GLOB Source RELATIVE ${CMAKE_CURRENT_SOURCE_DIR} *.f90 *.F90 *.f *.F)
+
----------------
naromero77 wrote:
> Meinersbur wrote:
> > Isn't this redundant with what `llvm_singlesource` does anyway?
> Good question and I didn't know the answer until after I did a little bit of experimentation.
> 
> `Source` is an input variable into `llvm_singlesource()`, without it the target is built in the wrong directory and CMake complains.
> 
> The structure of `CMakeLists.txt` is similar to what is done elsewhere in the testsuite:
> https://github.com/llvm/llvm-test-suite/blob/master/SingleSource/UnitTests/C%2B%2B11/CMakeLists.txt#L4-L9
There's something wrong if this is not working without defining `Source`. See `cmake/modules/SingleMultiSource.cmake`:
```
  if(DEFINED Source)
    set(sources ${Source})
  else()
    file(GLOB sources *.c *.cpp *.cc *.f *.F *.f90 *.F90)
  endif()
```
i.e. if `Source` is undefined, it does the file search by itself. Defining `Source` should only be necessary if there are additional source files that should not be compiled. I don't know why `SingleSource/UnitTests/C++11` does it this way. The majority of other programs rely on that it does work.


Repository:
  rT test-suite

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

https://reviews.llvm.org/D93778