Skip to content
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

Don't pass wd4668 to clang-cl #1856

Merged
merged 1 commit into from
Nov 14, 2023
Merged

Conversation

roblabla
Copy link
Contributor

@roblabla roblabla commented Aug 5, 2023

Fixes #1843

We should only pass /wd4668 when the compiler is MSVC, not when it's clang-cl, as the later does not understand that flag (and the issue it fixes is not relevant to clang-cl-based builds).

@quyykk
Copy link

quyykk commented Sep 4, 2023

@sylvestre sorry for the ping, but sccache is currently unusable with clang-cl without this fix so it would be great to get this PR reviewed and merged. thanks! 😄

@sylvestre
Copy link
Collaborator

Sorry I missed it.
Could you please add a test to make sure we don't regress?

@aeubanks
Copy link

aeubanks commented Sep 5, 2023

the description is not quite right, see my comment in #1843 (comment)

but this approach is still right

we should also not pass -- for MSVC, but that's a separate bug

@glandium
Copy link
Collaborator

glandium commented Sep 5, 2023

we should also not pass -- for MSVC, but that's a separate bug

I'd argue it's not sccache's business to remove command line arguments that were passed in.

@codecov-commenter
Copy link

codecov-commenter commented Sep 7, 2023

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (7358e8c) 30.72% compared to head (0653933) 30.68%.

Files Patch % Lines
src/compiler/msvc.rs 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1856      +/-   ##
==========================================
- Coverage   30.72%   30.68%   -0.05%     
==========================================
  Files          51       51              
  Lines       19190    19191       +1     
  Branches     9237     9238       +1     
==========================================
- Hits         5897     5888       -9     
+ Misses       7758     7757       -1     
- Partials     5535     5546      +11     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@sylvestre sylvestre merged commit 2f1bf9b into mozilla:main Nov 14, 2023
38 of 39 checks passed
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.

C++ ClangCL on Windows - /wd4668 flag results in error
6 participants