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

[Diana Picus via Phabricator via flang-commits]

rovka 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)
----------------
awarzynski wrote:
> 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)
> ```
Makes sense :)


================
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})
----------------
awarzynski wrote:
> 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)
Cool, thanks!


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

https://reviews.llvm.org/D128260