[libc][windows] fix small build issues.

[michaelrj]

Summary

The windows build has fallen behind a little, this patch fixes some
issues that were preventing it from building.
Specifically: Some subfolders weren't being included, leading to missing
targets in the cmake.

[lntue]

libc/src/__support/FPUtil/x86_64/FEnvImpl.h

57 ↗

(On Diff #463022)

Can you also change other exception flags to the same format too?

[sivachandra]

libc/src/__support/FPUtil/x86_64/FEnvImpl.h

57 ↗

(On Diff #463022)

Where is the macro named OVERFLOW coming from?

[sivachandra]

libc/src/CMakeLists.txt

12 ↗

(On Diff #463022)

I think this is a case of inappropriate design somewhere - adding these directories for Windows incorrectly implies that they are available for Windows as well.

[michaelrj]

address comments

libc/src/CMakeLists.txt

12 ↗

(On Diff #463022)

as far as I can tell, the issue is coming from remove_test which is in test/src/stdio. It includes libc.src.unistd.access which doesn't exist unless the unistd folder is included. Putting that inside an "if target is linux" condition also works, but I feel like that is prone to mistakes for future targets that also need linux only pieces.

libc/src/__support/FPUtil/x86_64/FEnvImpl.h

57 ↗

(On Diff #463022)

here's the error:

In file included from C:/src/llvm-project/libc\src/__support/FPUtil/FEnvImpl.h:21:
C:/src/llvm-project/libc\src/__support/FPUtil/x86_64/FEnvImpl.h(271,44): error: expected unqualified-id
    raise_helper(internal::ExceptionFlags::OVERFLOW);
                                           ^
C:\Program Files (x86)\Windows Kits\10\include\10.0.19041.0\ucrt\corecrt_math.h(965,25): note: expanded from macro 'OVERFLOW'
    #define OVERFLOW    _OVERFLOW
                        ^
C:\Program Files (x86)\Windows Kits\10\include\10.0.19041.0\ucrt\corecrt_math.h(68,21): note: expanded from macro '_OVERFLOW'
#define _OVERFLOW   3   // overflow range error

If I had to guess it's coming in through #include <fenv.h>

[lntue]

libc/src/__support/FPUtil/x86_64/FEnvImpl.h

57 ↗

(On Diff #463022)

Look like that definition is guarded by _CRT_INTERNAL_NONSTDC_NAMES (https://github.com/tpn/winsdk-10/blob/master/Include/10.0.16299.0/ucrt/corecrt_math.h#L962) internally to Windows C runtimes? Either #undef this or adding some Windows specific build flag might be another solution?

51 ↗

(On Diff #463048)

This one also?

[michaelrj]

fix missed flag

[michaelrj]

libc/src/__support/FPUtil/x86_64/FEnvImpl.h

57 ↗

(On Diff #463022)

#undef OVERFLOW does work, but that feels like a dangerous solution

[lntue]

libc/src/__support/FPUtil/x86_64/FEnvImpl.h

57 ↗

(On Diff #463022)

What I meant is to detect if _CRT_INTERNAL_NONSTDC_NAMES is defined then #undef _CRT_INTERNAL_NONSTDC_NAMES before including <fenv.h> and maybe redefine it after for Windows.

[sivachandra]

libc/src/__support/FPUtil/x86_64/FEnvImpl.h

57 ↗

(On Diff #463022)

I thought these macros stay undefined when compiling with clang-cl. Does something from here help: https://clang.llvm.org/docs/MSVCCompatibility.html

[michaelrj]

split fenv changes to separate patch

libc/test/src/stdio/CMakeLists.txt

add_libc_unittest(
   SUITE
     libc_stdio_unittests
   SRCS
     puts_test.cpp
   DEPENDS
     libc.src.stdio.puts
 )
 
+if(${LIBC_TARGET_OS} STREQUAL "linux")
   add_libc_unittest(
     remove_test
     SUITE
       libc_stdio_unittests
     SRCS
       remove_test.cpp
     DEPENDS
       libc.include.errno
       libc.include.unistd
       libc.src.fcntl.open
       libc.src.stdio.remove
       libc.src.sys.stat.mkdirat
       libc.src.unistd.access
       libc.src.unistd.close
   )
+endif()
 
 add_subdirectory(printf_core)
 add_subdirectory(testdata)