Suppress warnings in LLVM wrapper when targeting MSVC #118177
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @cuviper (or someone else) soon. Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (
|
rustbot
added
S-waiting-on-review
This comment has been minimized.
This comment has been minimized.
|
I don't have much context with MSVC to know if these suppressions are appropriate -- maybe @ChrisDenton can comment? |
|
@sivadeilra what version of MSVC are you using? I don't see any warnings from (There are a lot of C deprecation warnings from |
|
I'm using VS 2022. The warnings are displayed only when compilation fails, because the warnings are treated as stdout of a But I'm doing development on the C++ code in rust_llvm right now. So whenever I build something and it fails due to a 1-line mistake on my part, I have to scroll through hundreds of lines of irrelevant warnings just to find the one line that is relevant to my work. I know, I could use a regex or something, but that just adds one more step to a workflow. |
|
Sorry for my slow response. These suppressions do look reasonable to me. It'd be great to actually fix the warnings properly (or else make suppression more targeted) but that seems like an upstream issue? Or if not then it'd be a lot of work. |
Ah, okay, that build-script hiding is the part I forgot.
Can we target it with pragma push/pop around the includes? |
I tried that, and I found that template instantiations triggered some of these warnings, even outside of the push scope. :( It would also require putting the push/pop in a lot more places. I think the real fixes should happen upstream in LLVM, but I'm not sure whether they'll take those changes since they're super focused on self-hosting Clang. They make sure that Clang compiles and works with MSVC, but they're fairly slow to take fixes that don't affect Clang. |
Thanks for trying! Then I think it's ok to accept this for now, and maybe future upstream fixes will let us tighten up again. @bors r+
If they are true problems, then I hope they'll be accepted -- but it may well be hard work to argue that the MSVC toolchain is right about these cases, and perhaps that Clang is too lax. Or it could just be conditional code that's only broken in the MSVC path, which should take less justification to clean up. (I'm guessing blindly here. 🙈) |
bors
added
S-waiting-on-bors
…iaskrgr Rollup of 6 pull requests Successful merges: - rust-lang#117981 (Remove deprecated `--check-cfg` syntax) - rust-lang#118177 (Suppress warnings in LLVM wrapper when targeting MSVC) - rust-lang#118317 (tip for define macro name after `macro_rules!`) - rust-lang#118504 (Enforce `must_use` on associated types and RPITITs that have a must-use trait in bounds) - rust-lang#118660 (rustc_arena: add `alloc_str`) - rust-lang#118681 (Fix is_foreign_item for StableMIR instance ) r? `@ghost` `@rustbot` modify labels: rollup
|
Thanks, much appreciated! |
Rollup merge of rust-lang#118177 - sivadeilra:suppress-llvm-warnings, r=cuviper Suppress warnings in LLVM wrapper when targeting MSVC The LLVM header files generate many warnings when compiled using MSVC. This makes it difficult to work on the LLVM wrapper code, because the warnings and errors that are relevant to local edits are obscured by the hundreds of lines of warnings from the LLVM Headers.
The LLVM header files generate many warnings when compiled using MSVC. This makes it difficult to work on the LLVM wrapper code, because the warnings and errors that are relevant to local edits are obscured by the hundreds of lines of warnings from the LLVM Headers.