Fix is_flag_supported for assembly-specific flags#1632
Fix is_flag_supported for assembly-specific flags#1632naoNao89 wants to merge 1 commit intorust-lang:mainfrom
Conversation
naoNao89
force-pushed
the
fix-issue-359-is-flag-supported-assembly
branch
3 times, most recently
from
5265c16 to
da402eb
Compare
NobodyXu
left a comment
NobodyXu
left a comment
There was a problem hiding this comment.
Thank you for this PR!
just have some small nits, and I'd like feedback from others to ensure that this work on all compilers we need to support
| ) | ||
| } | ||
|
|
||
| fn ensure_check_file(&self) -> Result<PathBuf, Error> { |
There was a problem hiding this comment.
It seems that msvc/cl.exe may not support compiling file from stdin? cc @ChrisDenton
There was a problem hiding this comment.
i have updated, to use a hybrid approach:
For MSVC: Keep the filebased compilati on (creates flag_check.c/cpp/cu files)
For GCC/Clang: Use stdin compilation with -x <lang> ( like -xassembler-with-cpp)
There was a problem hiding this comment.
Thanks that'd absolutely work for major use cases, though it'd be great if it's more like a dynamic fallback.
From the top of my head, the nvcc/sccache/ccache maybe a problem.
For nvcc, it seems that
nvcc -x cu -Is needed.
And sccache does not support it at all, it's possible to use sccache with this crate
- by specifying an env, or
- configuring cargo to use it
- symlink/hardlink sccache to cc/gcc/clang etc in /usr/local/bin
There was a problem hiding this comment.
It seems that msvc/cl.exe may not support compiling file from stdin? cc @ChrisDenton
That is correct. I don't if there's some clever way to trick it into doing so but I doubt it.
There was a problem hiding this comment.
Yes basically we can only do this for clang/gcc, if there's no wrapper applied.
And if it fails, fallback to temp file is required.
…ted() Changed flag validation from file-based to stdin-based compilation to support language-selection flags like -xassembler-with-cpp. This also improves flag validation by ensuring invalid flags are properly rejected. - Use stdin compilation with explicit language for most flags - For -x flags, let the flag determine the language - Add -Werror=unknown-warning-option for clang to reject invalid -W flags - Remove unused ensure_check_file() method - Add regression test for assembly flags
naoNao89
force-pushed
the
fix-issue-359-is-flag-supported-assembly
branch
from
da402eb to
bbc61bd
Compare
3 participants
Fixed a 7-year-old bug where
is_flag_supported()failed with assembly flags like-xassembler-with-cpp.The issue? It was testing flags against C files. Now it uses stdin compilation for GNU/Clang, avoiding file extension problems entirely.
Added regression test to prevent this from breaking again.
Fixes #359