Swift markdown support #69
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
These should be provided by the proper inclusion of `Windows.h`. This seems to currently properly function without the macros, so remove the extra macros.
This allows us to wire together the various build components. With this file available, we can wire together swift-markdown to cmark-gfm so that we can start building swift-format.
Add additional headers that are required for the GFM extensions library.
Add the modulemap for cmark_gfm_extensions and split up the installed headers as there are two separate modulemaps. This should hopefully enable us to build swift-markdown.
Original commit message: ~~~ build: add an upgrade path for newer CMake CMake 3.12 deprecated `FindPythonInterp`, and with CMake 3.27, were obsoleted with CMP0148. Add a version check and switch to the new behaviour to allow building with newer releases. When the minimum CMake version is increased, we can look at dropping the compatibility path. ~~~
Add missing include paths to be used when building this library in a larger CMake project. Add a missing module.modulemap for the extensions library. This is the first step towards enabling building swift-markdown with CMake which is to enable building swift-format via CMake for use in the LSP.
Modern versions of MSVC (2015+) support most of the C99 standard, including the `inline` keyword. Avoid the vendor extension `__inline` and use the ISO standard spelling.
compnerd
force-pushed
the
swift-markdown-support
branch
from
28e25f8
to
3cd4154
Compare
Use the compiler provided macros to check if we are on a Unix system. `unistd.h` is the Unix standard header which must be provided by any Unix system. Apple macOS X is special in that it does not define `__unix__` but is a Unix system. Sink this into the code rather than keeping it in the build system to allow us to remove the generated configuration headers.
This removes the workaround for pre-VS2015 compilers which did not support sufficient amounts of the C99 standard. As VS0213 is now no longer supported, remove the workaround as we can expect users to be at supported Visual Studio release (VS2019+).
Remove shims for `snprintf` and `vsnprintf` which were unavailable prior to VS2015. As VS2013 is no longer serviced, this reduces complexity in the project.
Use __has_builtin to check at compile time if the feature is supported. This macro is supported by both clang and GCC (as of 10). GCC 10 was released ~2020 [1], which is ~4 years old now. In the case that the compiler in use is not new enough, we still provide the fallback so that the code will compile but without the additional hints for the branch probability (which also tends to be accounted for by modern branch predictors). This removes the final use of the config.h header and the configure time checks. Remove the use of config.h from the code base. [1] https://gcc.gnu.org/releases.html
These were not in use, so avoid including them which should speed up the CMake configure phase.
`stdbool.h` is part of C99 however was not provided by Visual Studio 2013 until RTM [1]. Remove the check for the header and inline the include at the usage sites rather than relying on `config.h`. VS2013 was EOL'ed Apr 9, 2019, with extended support ending Apr 9, 2024. [1] https://devblogs.microsoft.com/cppblog/c99-library-support-in-visual-studio-2013/
Policy settings may impact how `project` functions. They should be set immediately after `cmake_minimum_required` (which implicitly sets policies). Use the `POLICY` check to see if a policy is defined rather than using a version check.
CMake 3.15+ remove `/W3` from the language flags under MSVC with CMP0092. Set the policy to new to avoid the D9025 warning.
Custom local modules are expected to reside in `cmake/modules`. Relocate them and update the top-level CMakeLists.
Include the necessary support libraries uniquely to avoid re-processing the files in CMake.
man pages are extremely useful, but are not generally available on Windows. This changes the install condition to check for the Windows cross-compile rather than the toolchain in use. It is possible to build for Windows using clang in the GNU driver.
Move all the flags to the top-level CMakeLists and apply them conditionally based on the language. Avoid mucking with the user-only `CMAKE_C_FLAGS`.
`BUILD_TESTING` is the CMake sanctioned option for the control of tests (c.f. https://cmake.org/cmake/help/latest/module/CTest.html). Use the standard option to control the build of test targets.
Handle the test addition where the test is defined. Use generator expressions to simplify the path adjustment on Windows.
Do not create a fake test to indicate that there are no tests, simply do nothing.
This was obsoleted a long time ago, remove the now vestigial variable.
Use the proper generator expression for the python interpreter and adjust a typo in the component name.
Use generator expressions to compute the path rather than hardcoding the layout. Finally, clean up some unnecessary quoting and uniformly spell commands in the tests.
Match the style as recommended by CMake documentation in [1]. This makes it easier to read the command being invoked when running tests. No functional change. [1] https://cmake.org/cmake/help/latest/command/add_test.html
These settings are applied globally, hoist them to the top level of the project to make them easier to spot.
~~~ S:\SourceCache\apple\swift-cmark\api_test\main.c(1391): warning C4152: nonstandard extension, function/data pointer conversion in expression ~~~
Variables should be avoided. Repetition is not bad in this case. Inline the single use variables.
This uses the CMake mechanism for including the current source and binary directories. This avoids the custom handling for this.
Prefer to use the standard `BUILD_SHARED_LIBS` option to control the library type that we are building. Build a single instance of the library. This allows for better control of the library usage in a larger project. See the following blog post for the recommendation: https://alexreinking.com/blog/building-a-dual-shared-and-static-library-with-cmake.html
Repetition is preferred. This aids in discoverability when working with the build system.
Avoid setting properties individually and set them in bulk. This avoids unnecessary property computation in CMake.
The SOVERSION was the same as the VERSION parameter, just use the same parameter.
The includes are properly propagated by the targets, there is no need to explicitly specify them.
This macro does not impact the public API surface and the user should not have to care. This removes the macro from the configuration headers.
This header is an internal header and should not be exposed to the user. This fully internalises the concept of thread safety to the build.
These no longer serve any purpose and allow us to remove them.
We had drifted in the version management. Rather than risking the divergence between CMake and SPM in the future, prefer to just have a single manually maintained version of the version header.
Shuffle the behaviour checks to the beginning of the processing. Co-locate and hoist the options that the project supports, put custom behavioural changes in a single location.
compnerd
force-pushed
the
swift-markdown-support
branch
from
3cd4154
to
c5d1389
Compare
When building a Swift target against this library we need to ensure that we pass along `-DCMARK_GFM_STATIC_DEFINE` to the Clang Importer. This is required to build SwiftFormat against this library statically.
compnerd
force-pushed
the
swift-markdown-support
branch
from
68aa543
to
33410d5
Compare
This file should be excised but retain it temporarily to allow us to migrate away from the current build infrastructure.
This header does not cover all headers. Furthermore, it forces the subsuming of `mutex.h` which is a private header. Fixing the module definition to repair the macOS build.
bnbarham
reviewed
Jan 23, 2024
There was a problem hiding this comment.
This all seems fine to me, but it would be good to get an approval from a swift-cmark maintainer (@QuietMisdreavus 🙏 ) before merging.
Note that cross-repo toolchain testing has already been completed: swiftlang/swift#70791 (thanks!).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In order to support a shared build for swift-markdown and the swift frontend, we need to clean up the build to allow us to generate the necessary harness for the build and to control the build. In the process, clean up the build and pull some changes from upstreams.