Fix support for clang cl-mode driver #1069
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #1069 +/- ##
===========================================
+ Coverage 85.26% 85.48% +0.21%
===========================================
Files 305 307 +2
Lines 23684 23850 +166
===========================================
+ Hits 20193 20387 +194
+ Misses 3491 3463 -28 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
mizvekov
force-pushed
the
GH904
branch
5 times, most recently
from
87a66f2 to
b99b52c
Compare
|
An automated preview of the documentation is available at https://1069.mrdocs.prtest2.cppalliance.org/index.html If more commits are pushed to the pull request, the docs will rebuild at the same URL. 2025-10-16 18:30:41 UTC |
| // ------------------------------------------------------ | ||
| // Target architecture | ||
| // ------------------------------------------------------ | ||
| constexpr auto is_target_option = [](std::string_view opt) { |
There was a problem hiding this comment.
Just out of curiosity, isn't this logic needed anymore (IIRC we had to include this to make it work on macOS), or is this logic reimplemented somewhere else?
There was a problem hiding this comment.
It's not reimplemented. Nothing broke without it as far as I know.
The specific problem for CL-mode here is that, when a target is specified which is different than the MSVC one, then the CL driver ignores a bunch of flags.
| { | ||
| new_cmdline.emplace_back("-std=c++23"); | ||
| new_cmdline.emplace_back( | ||
| is_clang_cl ? "-std:c++23preview" : "-std=c++23"); |
There was a problem hiding this comment.
I may not recall our previous discussions. Do these flags really mean the same?
There was a problem hiding this comment.
The CL-mode driver only supports the "std:" spelling instead of "std=".
Just like MSVC, it interchangeably supports either "-" or "/" as a flag introducer.
And MSVC only recognizes the "c++23preview" option, even in the latest MSVC available in compiler explorer.
Clang follows suit here and doesn't recognize just "c++23" either.
| else | ||
| { | ||
| new_cmdline.emplace_back("-std=c23"); | ||
| new_cmdline.emplace_back(is_clang_cl ? "-std:c17" : "-std=c23"); |
There was a problem hiding this comment.
Do they mean the same thing? Maybe, if that's possible, this case and the case above would be a good case for using that flag that bypasses the clang-cl conversions? Because I'm not sure someone else would understand exactly why these mean the same thing. In other words, it looks like code someone will want to change in passing.
There was a problem hiding this comment.
No they don't mean the same thing, these are different C standards, we should use the same regardless, I will fix this.
There was a problem hiding this comment.
Ah I see, CL doesn't have a c23 option because MSVC doesn't either.
I think we could come up with a different approach here, we could switch to using the latest standard, or maybe not doing this at all, and relying on the user trusting the compiler default, or specifying his own.
There was a problem hiding this comment.
Changed to use the latest standard for both.
4 participants
Fixes #904