Skip to content

5.9.0 Windows macro support #68335

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 48 commits into from

Conversation

compnerd
Copy link
Member

@compnerd compnerd commented Sep 5, 2023

This backports the changes to support macros on Windows. The bulk of the changes are to the tests, the C++ and runtime pieces fix mismatches between operator new/malloc and free and implement Windows process execution support.

@compnerd compnerd requested a review from a team as a code owner September 5, 2023 22:52
@compnerd
Copy link
Member Author

compnerd commented Sep 5, 2023

@swift-ci please test

@compnerd
Copy link
Member Author

compnerd commented Sep 5, 2023

Please test with following PRs:
swiftlang/llvm-project#7400

@swift-ci please test

1 similar comment
@compnerd
Copy link
Member Author

compnerd commented Sep 6, 2023

Please test with following PRs:
swiftlang/llvm-project#7400

@swift-ci please test

compnerd and others added 26 commits September 7, 2023 07:24
`CMAKE_SHARED_LIBRARY_PREFIX` is empty on Windows.  This accounts for
that and quotes the parameter which we otherwise will see as an error
due to insufficient parameters being passed.
This begins reworking the API to be less POSIX centric and more
generally usable.  The API was defined in terms of `dlopen`/`dlsym`
rather than the better suited `llvm::sys::DynamicLibrary` APIs which
would avoid most of the work that needs to be done here for platform
specifics.
Include `stdint.h` for non-MSVC compilers.
We cannot depend on Swift's unsafe pointers using `malloc` under the
hood, so don't use `free` on them. Instead, expose entry points from
Swift to C++ to free any Swift-allocated data structures that need to
be deallocated from C++.
Buyer beware: the `MallocAllocator` does not, in fact, use `malloc`, so
one cannot pair an allocation using `MallocAllocator` with an actual
`free`.
The C type tha corresponds to Swift's pointer-sized `Int` and `UInt`
types varies from one platform to the next. The canonical C types are
`ptrdiff_t` and `size_t`, but we're in the depths of the compiler we
can't include the C library headers that provide them because they
introduce cyclic module dependencies. Sigh.

SwiftShims has some logic to compute these types. However, SwiftShims
is part of the Swift runtime, not the compiler, so those headers
cannot be included here. So, we clone the logic and simplify it
somewhat for our use case.

This fixes truncation issues on Windows, where the uses of
`unsigned long` and `long` for Swift(U)Int are incorrect.
…t/UInt

64-bit Windows defines both _WIN64 and _WIN32, so the logic here would
always end up defining 32-bit C types for Swift's `Int` and `UInt`.
Fix the ordering to check for 64-bit first, then 32-bit second.

Note that the SwiftShims version of this code has always been wrong,
but it's completely benign because SwiftShims is only used in the
Swift runtime itself, which is built with Clang (on all platforms),
and doesn't need to go through this code path. Still, we fix it in both
places, so we don't get a nasty surprise if the SwiftShims version of
the header later gets included in a non-Clang C++ compiler.
Co-authored-by: Saleem Abdulrasool <[email protected]>
Co-authored-by: Saleem Abdulrasool <[email protected]>
The type definitions do not uniformly import as `Swift.Int` and `Swift.UInt`.  Add some casts to accommodate the type conversions.
Implement process launching on Windows to support macros.  Prefer to use
the LLVM types wherever possible.  The pipes are converted into file
descriptors as the types are internal to the process.  This allows us to
have similar paths on both sides and avoid having to drag in `Windows.h`
for the definition of `HANDLE`.  This is the core missing functionality
for Windows to support macros.
Windows supports `\` and `/` as path separators.  Account for both in
computing the basename.  This repairs a few test failures on Windows.
There is no real reason to split the behaviour here as this is a file
system primitive.  Special case Apple and Linux platforms to address
feedback.
Windows paths involve a `:`.  Ideally, we would just use `;` as the
separator rather than `:`, but for now just limit the split count to 2.
Replace the process_fine_grained_swiftdeps.sh with a python equivalent
(which also preserves the horrendous handling of YAML and even
"faithfully" replicates the horrible global variables).  This enables a
number of tests on Windows although the instigating macro test is not
yet enabled due to the need for further tweaks to the tests.
The `env` is required to set the environment variable as executing with
environment variables is a SysV Shell thing and is not generally
portable.  This is required to support Windows.
Replace the use of `tee` with proper redirection for the output.  This
reduces the dependency on Unix tools for portability.
Use this to define the macro location rather than the "host" dir (which
is actually for the build and not the host).  Furthermore, on Windows,
the build dir is /usr/lib/swift as the host content is in the SDK.

This prepares the tests for Windows.
compnerd and others added 21 commits September 7, 2023 07:24
Due to the fact that we do not statically link the toolchain, we cannot
currently support this test.  Mark it as unavailable currently with the
early syntax parser enabled.
The paths are not handled uniformly which makes it difficult to match
the build directory pattern.  This is needed to handle the tests on
Windows.
This is a platform specific option and should be handled by lit patterns
rather than encoded into the tests.
Introduce a helper for mock plugins as `-rpath` is not a portable
construct.  This allows running more of the macro tests on Windows.
Fix an incorrect alteration to the test
The plugin layouts are different across platforms.  Move this into a
virtual method and allow replacement.  On Windows, the plugins are
placed into the `bin` directory as the DLLs should always be co-located
to ensure that the proper DLLs are found (there is no concept of RPATH).
A thrown error is stored as `any Error` which does not conform to
`CustomStringConvertible`.  When we perform the conversion via the
`String(describing:)` initialiser, for some reason the runtime does not
find the conformance and the error is not translated properly.
Explicitly casting to `PluginError` resolves the missing conformance and
fixes a test failure.
Account for import libraries and the associated layout difference on
platforms (e.g. DLLs are placed in `bin`).  This is required to enable
building the macro path on Windows.
This wires up the new macro properly into the build machinery to ensure
that the `distribution` target properly builds and installs the
dependencies.  This fixes the missing `swift-plugin-server` on Windows.
Add missing include for `atexit`.
@compnerd
Copy link
Member Author

compnerd commented Sep 7, 2023

Please test with following PRs:
swiftlang/llvm-project#7405
swiftlang/swift-syntax#2162
swiftlang/swift-driver#1436

@swift-ci please test

This fixes the test suite when performing a clean build.  The libraries
were being copied to the wrong location after the generalisation changes
made earlier.
@compnerd
Copy link
Member Author

Please test with following PRs:
swiftlang/llvm-project#7405
swiftlang/swift-syntax#2162
swiftlang/swift-driver#1436

@swift-ci please test

@compnerd
Copy link
Member Author

Please test with following PRs:
swiftlang/llvm-project#7405
swiftlang/swift-syntax#2162
swiftlang/swift-driver#1436

@swift-ci please test Linux platform

@compnerd compnerd closed this Oct 6, 2023
@compnerd compnerd deleted the 5.9.0-macros branch October 6, 2023 14:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants