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

build.rs: Use cc-rs's built-in logic for warnings-as-errors. #1343

Merged
merged 1 commit into from
Aug 13, 2021

Conversation

briansmith
Copy link
Owner

Take a step towards supporting Clang as the compiler for -msvc targets.

Take a step towards supporting Clang as the compiler for -msvc targets.
@briansmith briansmith self-assigned this Aug 13, 2021
@briansmith
Copy link
Owner Author

@awakecoding @Alovchin91 Please review this. This is one of many changes I'm making to officially support Clang for -msvc targets.

@awakecoding
Copy link
Contributor

It looks good to me, I guess cc automatically detects the compiler and sets the correct flags. Is there a way to tell cc to use clang/MSVC and not the regular MSVC compiler? And if so, does it use clang-cl (same flags as cl.exe) or clang.exe (same flags as clang on all platforms)? Did you have a way to check which compiler was picked up by cc in the end?

@codecov
Copy link

codecov bot commented Aug 13, 2021

Codecov Report

Merging #1343 (3adb0ca) into main (56561bc) will decrease coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1343      +/-   ##
==========================================
- Coverage   92.83%   92.82%   -0.01%     
==========================================
  Files         117      117              
  Lines       17826    17826              
  Branches      195      195              
==========================================
- Hits        16548    16547       -1     
- Misses       1244     1245       +1     
  Partials       34       34              
Impacted Files Coverage Δ
src/aead/less_safe_key.rs 95.68% <0.00%> (-0.87%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 56561bc...3adb0ca. Read the comment docs.

@briansmith
Copy link
Owner Author

It looks good to me, I guess cc automatically detects the compiler and sets the correct flags. Is there a way to tell cc to use clang/MSVC and not the regular MSVC compiler?

$ which clang
/c/apps-x86/Microsoft Visual Studio/2019/BuildTools/VC/Tools/Llvm/bin/clang
$ clang --version
clang version 12.0.0
Target: i686-pc-windows-msvc
Thread model: posix
InstalledDir: C:\apps-x86\Microsoft Visual Studio\2019\BuildTools\VC\Tools\Llvm\bin
$ CC=clang cargo -vv test --target=x86_64-pc-windows-msvc
$ CC=clang cargo -vv test --release --target=x86_64-pc-windows-msvc

And if so, does it use clang-cl (same flags as cl.exe) or clang.exe (same flags as clang on all platforms)?

If you use CC=clang it uses traditional clang flags. If you use CC=clang-cl then it uses MSVC flags. This will become apparent in the next PR in this series.

Did you have a way to check which compiler was picked up by cc in the end?

cargo -vv will show you the compiler invocations and cc-rs's logging.

Surprisingly, on Windows I couldn't get TARGET_CC to be respected; I had to use CC instead. :(

@briansmith
Copy link
Owner Author

The codecov job's failure is caused by there being less covered code, not by there be any new uncovered code, so it's benign (good even).

@briansmith briansmith merged commit b21ccc9 into main Aug 13, 2021
@briansmith briansmith deleted the b/warnings-are-errors branch August 13, 2021 03:16
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.

2 participants