-
Notifications
You must be signed in to change notification settings - Fork 333
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
Support clang 18 in init-compiler.sh #14572
Conversation
It got released recently. lld also started supporting s390x in that release.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thank you!
clang 18 introduces `-Wswitch-default`, which requires that every switch must have a `default` branch. We can fix that, but the other option `-Wcovered-switch-default` complains if all the cases in a switch are exhaustive and `defualt` doesn't do anything. So disable one of these mutually exclusive warnings. We will also need to merge in the changes from dotnet/arcade#14572 to actually try and use clang-18/clang++-18.
clang 18 introduces `-Wswitch-default`, which requires that every switch must have a `default` branch. We can add missing `default` in switches, but the other option `-Wcovered-switch-default` complains if all the cases in a switch are exhaustive and `default` doesn't do anything. So disable one of these mutually exclusive warnings. We will also need to merge in the changes from dotnet/arcade#14572 to actually try and use clang-18/clang++-18.
clang 18 introduces `-Wswitch-default`, which requires that every switch must have a `default` branch. We can add missing `default` in switches, but the other option `-Wcovered-switch-default` complains if all the cases in a switch are exhaustive and `default` doesn't do anything. So disable one of these mutually exclusive warnings. We will also need to merge in the changes from dotnet/arcade#14572 to actually try and use clang-18/clang++-18.
clang 18 introduces `-Wswitch-default`, which requires that every switch must have a `default` branch. We can add missing `default` in switches, but the other option `-Wcovered-switch-default` complains if all the cases in a switch are exhaustive and `default` doesn't do anything. So disable one of these mutually exclusive warnings. We will also need to merge in the changes from dotnet/arcade#14572 to actually try and use clang-18/clang++-18.
@@ -63,7 +63,7 @@ if [ -z "$CLR_CC" ]; then | |||
# Set default versions | |||
if [ -z "$majorVersion" ]; then | |||
# note: gcc (all versions) and clang versions higher than 6 do not have minor version in file name, if it is zero. | |||
if [ "$compiler" = "clang" ]; then versions="17 16 15 14 13 12 11 10 9 8 7 6.0 5.0 4.0 3.9 3.8 3.7 3.6 3.5" | |||
if [ "$compiler" = "clang" ]; then versions="18 17 16 15 14 13 12 11 10 9 8 7 6.0 5.0 4.0 3.9 3.8 3.7 3.6 3.5" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@janvorli @akoeplinger this hard-coded list has caused issues with source-build a few times when the distro has a version clang that is not yet in this list. Even on already released branches, we've had to update this list.
I don't know the origins of the list, but may be we can change the logic so it accepts whatever is the version is on the system. If there are some specific version that are known to have issues, we can error out for those.
cc @omajid
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Originally, there was no list and the build.sh in the dotnet/coreclr repo had a list of command line options for each clang version we knew was working. First, it was to filter out old versions that were not able to compile our code correctly (clang 3.4 and older). It was not clear at that time if all new versions will work correctly, so we kept the list. Then it was transferred to the current form. The idea was that we would add compilers to the list only after we verify that they produce correct code for the runtime so that people trying to build the runtime on whatever systems they have would not hit build / runtime problems. In the past, it was me verifying everything and then adding a new version here or giving it a green flag.
But maybe the benefit of curating this list is not that big and the pain in trying to build on systems with newer compilers is worse. So I guess I'd be fine with either removing the list or keeping it and just showing a warning message if the compiler is not on this list telling people there may be issues.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, sounds good
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd be fine with either removing the list or keeping it and just showing a warning message if the compiler is not on this list telling people there may be issues.
Of these options, I have a slight preference for the first as it avoids all maintenance of the list.
I've created an issue for this: #14632.
I'll pick it up when I find a moment for it, unless someone else picks it up sooner.
This is a targeted backport from a few other PRs that makes it possible to build dotnet/runtme's 8.0 branch on Fedora 40 which includes clang 18. - dotnet/arcade#14572 - dotnet#94782 - dotnet#99811
This is a targeted backport from a few other PRs that makes it possible to build dotnet/runtme's 8.0 branch on Fedora 40 which includes clang 18. - dotnet/arcade#14572 - #94782 - #99811
This is a targeted backport from a few other PRs that makes it possible to build dotnet/runtme's 6.0 branch on Fedora 40 which includes clang 18. - dotnet/arcade#14572 - dotnet#94782 - dotnet#99811
This is a targeted backport from a few other PRs that makes it possible to build dotnet/runtme's 6.0 branch on Fedora 40 which includes clang 18. - dotnet/arcade#14572 - dotnet#94782 - dotnet#99811
This is a targeted backport from a few other PRs that makes it possible to build dotnet/runtme's 6.0 branch on Fedora 40 which includes clang 18. - dotnet/arcade#14572 - #94782 - #99811
It got released recently. lld also started supporting s390x in that release.