[flang-commits] [PATCH] D128260: [Fortran] Ignore whitespace in FCVS test results

[Andrzej Warzynski via Phabricator via flang-commits]

awarzynski added inline comments.


================
Comment at: Fortran/UnitTests/fcvs21_f95/CMakeLists.txt:36-42
+# Tests 905 and 907 use list-directed output, for which the standard allows some
+# flexibility. Treat them separately.
+set(SPECIAL_CASES "FM905.f" "FM907.f")
+set(Source ${SPECIAL_CASES})
+set(FP_IGNOREWHITESPACE ON)
+llvm_singlesource()
+set(Source)
----------------
This is basically splitting the tests into 2 groups, which are processed independently. I think that it would help to have this highlighted a bit more. Perhaps something like this:
```
# GROUP 1:
# Tests 905 and 907 use list-directed output, for which the standard allows some flexibility with regards to how many white space characters to print. For these tests, we ignore whitespaces when comparing the output. 
set(SPECIAL_CASES "FM905.f" "FM907.f")
set(Source ${SPECIAL_CASES})

set(FP_IGNOREWHITESPACE ON)
llvm_singlesource()

# GROUP 2:
# Generic case.
set(Source)
```


================
Comment at: cmake/modules/SingleMultiSource.cmake:108-117
     if(EXISTS ${CMAKE_CURRENT_SOURCE_DIR}/${name}.reference_output.${ENDIAN}-endian.${SIZE_SUFFIX})
       set(REFERENCE_OUTPUT ${name}.reference_output.${ENDIAN}-endian.${SIZE_SUFFIX})
     elseif(EXISTS ${CMAKE_CURRENT_SOURCE_DIR}/${name}.reference_output.${SIZE_SUFFIX})
       set(REFERENCE_OUTPUT ${name}.reference_output.${SIZE_SUFFIX})
     elseif(EXISTS ${CMAKE_CURRENT_SOURCE_DIR}/${name}.reference_output.${ENDIAN}-endian)
       set(REFERENCE_OUTPUT ${name}.reference_output.${ENDIAN}-endian)
     elseif(EXISTS ${CMAKE_CURRENT_SOURCE_DIR}/${name}.reference_output.${TARGET_OS})
----------------
rovka wrote:
> kiranchandramohan wrote:
> > I am not pushing for this option. But this bit here seems to allow for reference output based on TARGET, ENDIAN-NESS etc.
> Right, I guess we could add some logic here based on COMPILER_ID or something of the sort. 
+1 I'm OK to revisit this once it becomes a problem (as opposed to expanding this patch)


Repository:
  rT test-suite

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

https://reviews.llvm.org/D128260