Install clang-scan-deps tool

[nolange]

Should be pretty safe changes. (did not want to make a separate PR for the macro update)

[mstorsjo]

Should be pretty safe changes. (did not want to make a separate PR for the macro update)

Please don't mix unrelated changes in the same PR. Even if it seems safe, they can have entirely separate issues, warranting separate discussions with different conclusions.

Re using a non-deprecated define for UWP - mingw-w64 headers don't have a single mention of WINAPI_FAMILY_PC_APP, so I don't think that works there. (The whole machinery for api families in mingw-w64 headers is a bit old and hasn't kept up with all the relevant changes in WinSDK - many of the bits in how it is implemented match the first iteration of this from WinSDK 8.0. A later WinSDK update refactored the internals for how it works, which IIRC the mingw-w64 implementation hasn't kept up with that.)

This shows up in the test log, but apparently my smoke test suite didn't catch it properly. See https://github.com/mstorsjo/llvm-mingw/actions/runs/8257442431/job/22594726733?pr=412. The uwp-error testcase, which usually fails like this, https://github.com/mstorsjo/llvm-mingw/actions/runs/8257612173/job/22595195864:

i686-w64-mingw32uwp-clang ../uwp-error.c -o .tested.build.uwp-error-mingw32uwp.exe -Wimplicit-function-declaration -Werror
../uwp-error.c:23:16: error: call to undeclared function 'GetOEMCP'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
   23 |     UINT res = GetOEMCP();
      |                ^
1 error generated.
OK: UWP build failed intentionally

Now failed like this:

In file included from ../uwp-error.c:19:
In file included from D:/a/_temp/msys64/llvm-mingw/include/windows.h:69:
In file included from D:/a/_temp/msys64/llvm-mingw/include/windef.h:21:
D:/a/_temp/msys64/llvm-mingw/include/winnt.h:308:11: error: unknown type name 'CONST'
  308 |   typedef CONST WCHAR *LPCWCH,*PCWCH;
      |           ^
[...]
fatal error: too many errors emitted, stopping now [-ferror-limit=]
20 errors generated.
OK: UWP build failed intentionally

The smoke testsuite does have a test for compiling an executable in UWP mode, which is supposed to work, but for that I only compiled the plain hello.c which doesn't include windows.h in any form, so that one does succeed. I'll amend it to try to building another test executable that does include and use some Windows APIs in UWP mode as well. I'm running a test to confirm that it would catch this issue sooner, see https://github.com/mstorsjo/llvm-mingw/compare/master...uwp-define.

[mstorsjo]

Also, re the libcxx modules, that change in itself looks ok, but if we're going to do that, we'll probably need to do a bit more.

I tried playing with a little, but will need to look further into it later.

The libc++ module files get installed into <root>/<arch>-w64-mingw32/share/libc++/v1 - but we could also want to install them into e.g. <root>/share/libc++/v1 instead. That's easy to achieve by adding -DLIBCXX_INSTALL_MODULES_DIR="$PREFIX/share/libc++/v1" to the build script. But <roo>/<arch>-w64-mingw32/lib/libc++.modules.json contains a relative path to this directory, and that breaks (it gets an incorrect relative path) if we specify LIBCXX_INSTALL_MODULES_DIR as an absolute path. (This should be easy to fix in libcxx though.)

If we're going to use modules, I presume we should ship clang-scan-deps as well (as AFAIK tools like cmake will need to use that), so we should filter it out in strip-llvm.sh. (I guess that goes for C++20 modules overall, not specifically libc++ as a module.)

I'm looking at https://discourse.llvm.org/t/llvm-discussion-forums-libc-c-23-module-installation-support/77087/16 as standalone example of how to use the libcxx modules. (AFAIK CMake integration to use it doesn't quite exist yet.)

I'm also looking at https://www.kitware.com/import-cmake-the-experiment-is-over/ as example for using C++20 modules in general.

[nolange]

I will lower my head in shame.

Yes, I somehow missed that mingw has vastly different headers, I previously was sure I checked both the WindowsSDK aswell as MingW.

The modules seem more complex aswell, with clang-scan-deps being necessary.

I did read the CMake Article and planned to play with that, did appear to me that its "solved" with CMake. In case you need help with that, I added a toolchainfile. Its intended to be placed in share/llvm-x86_64-w64-mingw32_toolchainfile.cmake in the root of llvm-mingw toolchain, this hopefully should be enough to direct CMake to the right paths.

if(CMAKE_VERSION VERSION_GREATER_EQUAL "3.20")
  set(_prefix "${CMAKE_CURRENT_LIST_DIR}/../")
  cmake_path(ABSOLUTE_PATH _prefix NORMALIZE)
  set(_filename "${CMAKE_CURRENT_LIST_FILE}")
  cmake_path(GET _filename FILENAME _filename)
else()
  get_filename_component(_prefix "${CMAKE_CURRENT_LIST_DIR}/../" ABSOLUTE)
  get_filename_component(_filename "${CMAKE_CURRENT_LIST_FILE}" NAME)
endif()

string(REGEX REPLACE "^llvm[_-]\(.*\)[_-]toolchain.*" "\\1" _arch "${_filename}")
string(REGEX REPLACE "^.*-mingw32\(.*\)" "\\1" _mingwtype "${_arch}")
string(REGEX REPLACE "-.*" "" _arch "${_arch}")
unset(_filename)

# set(_prefix /opt/llvm-mingw/)
# set(_mingwtype uwp)

set(CMAKE_SYSTEM_NAME Windows)
if(_arch)
  set(CMAKE_SYSTEM_PROCESSOR ${_arch})
else()
  set(CMAKE_SYSTEM_PROCESSOR ${CMAKE_HOST_SYSTEM_PROCESSOR})
endif()

set(CMAKE_ASM_COMPILER "${_prefix}bin/clang")
set(CMAKE_C_COMPILER "${_prefix}bin/clang")
set(CMAKE_CXX_COMPILER "${_prefix}bin/clang++")
set(CMAKE_RC_COMPILER ${_prefix}bin/llvm-rc)

set(CMAKE_CXX_FLAGS_INIT "-stdlib=libc++")

foreach(_lang ASM C CXX)
  set(CMAKE_${_lang}_COMPILER_TARGET ${CMAKE_SYSTEM_PROCESSOR}-w64-mingw32${_mingwtype})
  if(_mingwtype STREQUAL "uwp")
    set(CMAKE_${_lang}_FLAGS_INIT "${CMAKE_${_lang}_FLAGS_INIT} -D_WIN32_WINNT=0x0A00 -DWINVER=0x0A00 -DWINAPI_FAMILY=WINAPI_FAMILY_APP -DUNICODE -D_UCRT")
  endif()
endforeach()
unset(_lang)

set(_linker lld)
# set(_linker_extra " -Wl,--undefined-version")
if(_mingwtype STREQUAL "uwp")
  set(_linker_extra "${_linker_extra} -Wl,-lwindowsapp -Wl,-lucrtapp")
endif()
set(CMAKE_EXE_LINKER_FLAGS_INIT "--start-no-unused-arguments -stdlib=libc++ -fuse-ld=${_linker} -rtlib=compiler-rt -unwindlib=libunwind${_linker_extra} --end-no-unused-arguments")
set(CMAKE_MODULE_LINKER_FLAGS_INIT "${CMAKE_EXE_LINKER_FLAGS_INIT}")
set(CMAKE_SHARED_LINKER_FLAGS_INIT "${CMAKE_EXE_LINKER_FLAGS_INIT}")
set(CMAKE_LINKER ${_prefix}bin/ld.${_linker})
unset(_linker_extra)
unset(_linker)

set(CMAKE_SYSROOT "${_prefix}${CMAKE_SYSTEM_PROCESSOR}-w64-mingw32")

unset(_mingwtype)
unset(_prefix)

set(ENV{PKG_CONFIG_SYSROOT_DIR} "${CMAKE_SYSROOT}")
if($ENV{PKG_CONFIG_LIBDIR})
    set(ENV{PKG_CONFIG_LIBDIR} "${CMAKE_SYSROOT}/usr/lib/pkgconfig:${CMAKE_SYSROOT}/usr/share/pkgconfig:$ENV{PKG_CONFIG_LIBDIR}")
else()
    set(ENV{PKG_CONFIG_LIBDIR} "${CMAKE_SYSROOT}/usr/lib/pkgconfig:${CMAKE_SYSROOT}/usr/share/pkgconfig")
endif()

set(CMAKE_FIND_ROOT_PATH_MODE_PROGRAM NEVER)
set(CMAKE_FIND_ROOT_PATH_MODE_LIBRARY ONLY)
set(CMAKE_FIND_ROOT_PATH_MODE_INCLUDE ONLY)

[nolange]

Well.... guess not.

No builtin support for import std; or other compiler-provided modules.

https://cmake.org/cmake/help/latest/manual/cmake-cxxmodules.7.html#manual:cmake-cxxmodules(7)

[mstorsjo]

Well.... guess not.

No builtin support for import std; or other compiler-provided modules.

https://cmake.org/cmake/help/latest/manual/cmake-cxxmodules.7.html#manual:cmake-cxxmodules(7)

Yep, exactly. So until this is available in some form so we can test it, I think it's best to hold off of merging anything to master, before we've made sure that whatever layout we choose actually works with cmake somewhat out of the box. But until then we can certainly prepare things in a branch.

General C++20 modules with one's own code, as in that linked CMake article, should kinda work though - the only thing it requires probably is that we'd keep the clang-scan-deps executable, and we could probably merge that without waiting for the rest.

[mstorsjo]

Well.... guess not.

No builtin support for import std; or other compiler-provided modules.

https://cmake.org/cmake/help/latest/manual/cmake-cxxmodules.7.html#manual:cmake-cxxmodules(7)

@huangqinjin noted in https://github.com/mstorsjo/msvc-wine/pull/126 that the upcoming CMake 3.30 should have support for this.

I pushed a branch https://github.com/mstorsjo/llvm-mingw/commits/libcxx-modules which enables installing the libc++ module, and installing it into a directory that allows sharing the files across the architectures. (Since https://github.com/llvm/llvm-project/commit/0cd4babe0342c994d28916ecbbb7cd2b91023b9b, since LLVM 18.1.3, specifying a custom LIBCXX_INSTALL_MODULES_DIR like this should work.) So @huangqinjin, if you have time to try it out with the bleeding edge CMake, the toolchains built in https://github.com/mstorsjo/llvm-mingw/actions/runs/8718827994 (once it completes) should include the necessary files.

[huangqinjin]

I tested the toolchain, there are two problems.

1. CMake cannot find clang-scan-deps, see https://gitlab.kitware.com/cmake/cmake/-/blob/v3.29.0/Modules/Compiler/Clang-FindBinUtils.cmake#L49. CMake requires prefixed tool names e.g. x86_64-w64-mingw32-clang-scan-deps. After manually creating a symlink, the toolchain can successfully build some example projects in repo https://github.com/huangqinjin/cxxmodules with CMake 3.28+.

2. libc++ std modules are incompatible with C headers shipped in the toolchain. Many errors like

   n file included from /opt/llvm-mingw/share/libc++/v1/std.cppm:240:
   /opt/llvm-mingw/share/libc++/v1/std/ctime.inc:20:14: error: using declaration referring to 'ctime' with internal linkage cannot be exported
   20 |   using std::ctime;
     |              ^
   /opt/llvm-mingw/x86_64-w64-mingw32/include/time.h:267:33: note: target of using declaration
   267 | static __inline char *__CRTDECL ctime(const time_t *_Time) { return _ctime64(_Time); 

   The MinGW C headers need to be fixed to be exportable, i.e. use inline instead of static.

[nolange]

thanks for testing, for 1., you could try using a cmake toolchainfile: https://github.com/mstorsjo/llvm-mingw/pull/412#issuecomment-1994529838

for 2., dropping static might give you some ABI issues. probably needs some c++ wrappers instead of using

[mstorsjo]

I tested the toolchain, there are two problems.

1. CMake cannot find clang-scan-deps, see https://gitlab.kitware.com/cmake/cmake/-/blob/v3.29.0/Modules/Compiler/Clang-FindBinUtils.cmake#L49. CMake requires prefixed tool names e.g. x86_64-w64-mingw32-clang-scan-deps. After manually creating a symlink, the toolchain can successfully build some example projects in repo https://github.com/huangqinjin/cxxmodules with CMake 3.28+.

Thanks, I see!

Does the target triple play any role in the invocation of clang-scan-deps, i.e. does this tool need to find the sysroot headers?

I'm wondering if plain symlinks are enough, or if we'd need to create a fullblown wrapper script/executable for this.

Some tools, like regular clang itself, can infer the target triple implicitly, when invoked via a symlink as <triple>-clang. If inferring the target triple is enough, and no other options are needed, this is enough. However on Windows, we don't use symlinks for these executables - so that would end up duplicating the clang-scan-deps.exe executable a couple times. As long as we link against libclang-cpp.dll and libLLVM-<version>.dll, the executable isn't huge (it seems to be around 175 KB in one build), but it's still a bit excessive. So for many such cases, I'm using https://github.com/mstorsjo/llvm-mingw/blob/master/wrappers/llvm-wrapper.c as a way to have a minimal-ish executable, 14 KB, that just executes the unprefixed executable - but that loses the implicit reading of the target prefix from a symlink. Then again, on Windows, the default target triple is a mingw triple anyway, so it probably would work just fine in any case.

OTOH, if the clang-scan-deps tool really is target agnostic, perhaps the CMake rule should be extended to look for an unprefixed clang-scan-deps executable as well?

2. libc++ std modules are incompatible with C headers shipped in the toolchain. Many errors like

n file included from /opt/llvm-mingw/share/libc++/v1/std.cppm:240:
/opt/llvm-mingw/share/libc++/v1/std/ctime.inc:20:14: error: using declaration referring to 'ctime' with internal linkage cannot be exported
   20 |   using std::ctime;
      |              ^
/opt/llvm-mingw/x86_64-w64-mingw32/include/time.h:267:33: note: target of using declaration
  267 | static __inline char *__CRTDECL ctime(const time_t *_Time) { return _ctime64(_Time); 

The MinGW C headers need to be fixed to be exportable, i.e. use inline instead of static.

Oh, I see, thanks! This will indeed be a significant amount of work. There are a number of different patterns used for inline wrappers in the mingw-w64 headers, and they can be quite fiddly to get right, but I guess something like static inline for C and plain inline for C++ might be the best here, e.g. like the macro set up here: https://github.com/mingw-w64/mingw-w64/blob/master/mingw-w64-headers/crt/_mingw_mac.h#L285-L295

[huangqinjin]

After testing clang-scan-deps with different target triples, I believe that clang-scan-deps cannot automatically find out the target triple, and -target option is a must.

test-deps.cpp

#if defined(__x86_64__)
import x64;
#elif defined(__i386__)
import x86;
#elif defined(__aarch64__)
import arm64;
#elif defined(__arm__)
import arm;
#endif

without -target

archs=(x86_64 i686 aarch64 armv7)

for a in ${archs[@]}; do
  for b in ${archs[@]}; do
    echo "$a $b"
    /opt/llvm-mingw/bin/$a-w64-mingw32-clang-scan-deps -format=p1689 -- /opt/llvm-mingw/bin/$b-w64-mingw32-clang++ -std=c++23 -c test-deps.cpp
  done
done

All triple combinations output same, i.e. depends on x64 module (logical-name in requires[]), it is obviously wrong.

{
  "revision": 0,
  "rules": [
    {
      "primary-output": "test-deps.o",
      "requires": [
        {
          "logical-name": "x64"
        }
      ]
    }
  ],
  "version": 1
}

with -target

archs=(x86_64 i686 aarch64 armv7)

for a in ${archs[@]}; do
  for b in ${archs[@]}; do
    echo "$a $b"
    /opt/llvm-mingw/bin/$a-w64-mingw32-clang-scan-deps -format=p1689 -- /opt/llvm-mingw/bin/$b-w64-mingw32-clang++ --target=$b-w64-mingw32 -std=c++23 -c test-deps.cpp
  done
done

``` x86_64 x86_64 { "revision": 0, "rules": [ { "primary-output": "test-deps.o", "requires": [ { "logical-name": "x64" } ] } ], "version": 1 } x86_64 i686 { "revision": 0, "rules": [ { "primary-output": "test-deps.o", "requires": [ { "logical-name": "x86" } ] } ], "version": 1 } x86_64 aarch64 { "revision": 0, "rules": [ { "primary-output": "test-deps.o", "requires": [ { "logical-name": "arm64" } ] } ], "version": 1 } x86_64 armv7 { "revision": 0, "rules": [ { "primary-output": "test-deps.o", "requires": [ { "logical-name": "arm" } ] } ], "version": 1 } i686 x86_64 { "revision": 0, "rules": [ { "primary-output": "test-deps.o", "requires": [ { "logical-name": "x64" } ] } ], "version": 1 } i686 i686 { "revision": 0, "rules": [ { "primary-output": "test-deps.o", "requires": [ { "logical-name": "x86" } ] } ], "version": 1 } i686 aarch64 { "revision": 0, "rules": [ { "primary-output": "test-deps.o", "requires": [ { "logical-name": "arm64" } ] } ], "version": 1 } i686 armv7 { "revision": 0, "rules": [ { "primary-output": "test-deps.o", "requires": [ { "logical-name": "arm" } ] } ], "version": 1 } aarch64 x86_64 { "revision": 0, "rules": [ { "primary-output": "test-deps.o", "requires": [ { "logical-name": "x64" } ] } ], "version": 1 } aarch64 i686 { "revision": 0, "rules": [ { "primary-output": "test-deps.o", "requires": [ { "logical-name": "x86" } ] } ], "version": 1 } aarch64 aarch64 { "revision": 0, "rules": [ { "primary-output": "test-deps.o", "requires": [ { "logical-name": "arm64" } ] } ], "version": 1 } aarch64 armv7 { "revision": 0, "rules": [ { "primary-output": "test-deps.o", "requires": [ { "logical-name": "arm" } ] } ], "version": 1 } armv7 x86_64 { "revision": 0, "rules": [ { "primary-output": "test-deps.o", "requires": [ { "logical-name": "x64" } ] } ], "version": 1 } armv7 i686 { "revision": 0, "rules": [ { "primary-output": "test-deps.o", "requires": [ { "logical-name": "x86" } ] } ], "version": 1 } armv7 aarch64 { "revision": 0, "rules": [ { "primary-output": "test-deps.o", "requires": [ { "logical-name": "arm64" } ] } ], "version": 1 } armv7 armv7 { "revision": 0, "rules": [ { "primary-output": "test-deps.o", "requires": [ { "logical-name": "arm" } ] } ], "version": 1 } ```

[huangqinjin]

I think all options to clang must be explicitly specified in command lines, custom wrappers over plain clang do not work with clang-scan-deps.

[mstorsjo]

Thanks for looking into this, and thanks for providing examples for how to use and investigate this.

With this in mind, it does indeed look like we'll need to provide a wrapper script/executable for clang-scan-deps in order to get this right. (It might, practically, mostly work for most cases even without it, but let's get this right from the start.)