ICU-23151 Fix building on Windows using msbuild with clang-cl. #3537
Conversation
This file contains hidden or 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
Add config all 4 windows platforms instead of just ARM. Store off CPU Arch for use with writeObjectCode Change checkCpuArchitecture to accept uppercase option values. Preserve lowercase option values for compatibility with previous versions in case people are using these options.
Removed browse information which isn't needed for modern Visual Studio.
|
|
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.
When building the latest release 77.1 on Windows from the command line I encountered two issues when using Clang-CL as a compiler (cl compiled fine). I have described them in the ticket, but to reiterate the first is a failure to generate sbr files in debug builds and the second is nullptr passed to strcmp when building data in x64 builds.
For the sbr files, I have removed browse information from the solution. If this is not an acceptable solution I am happy to revisit it, but my understanding is browse information isn't needed anymore per reading this documentation https://learn.microsoft.com/en-us/cpp/build/reference/creating-an-dot-sbr-file and the option could be fairly safely removed.
The second issue I fixed by piggy backing on the existing dll architecture option. This appears to currently specify an architecture for a building a data file instead of letting the system auto-detect it and seemed harmless to always perform. I Updated the code to store off this architecture, and pass it through to writeObjectCode allowing Clang-CL builds to generate the appropriate data file. I updated the data generation code to handle ARM in addition since that option was missing and couldn't be worse than the previous undefined behavior, and preserved support for lowercase options used by --cpu-arch with genccode. I did not add a new option as -c was already used, and having two different ways to specify the architecture seemed more bug prone, but I am open to suggestions.
I compiled the project using the 8 variants of: Win32/x64, Debug/Release, and MSVC/Clang-CL as well as ran the tests for all these configurations which passed. I also compiled all 8 of these variants to ensure Win32 Release Clang-CL as well as all 4 MSVC configurations passed without the change, while the other 3 Clang-CL variants failed to build.
I was not able to test ARM / ARM64 though invocations of the tool did not change in the makedata.mak scripts for these platforms. My reason for adding the ARM variant is it can't be worse than the previous code here which if reached would have had the same issue with strcmp.
Checklist