-
Notifications
You must be signed in to change notification settings - Fork 503
[msvc] Don't use -M* flags. #723
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
Conversation
src/lib.rs
Outdated
// interfere with the process. This can be achieved with | ||
// -MT -Zl, because object modules compiled with these flags | ||
// can be linked with either VCCRT version, static or dynamic, | ||
// debug or release... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be helpful to link the documentation here https://learn.microsoft.com/en-us/cpp/build/reference/zl-omit-default-library-name?view=msvc-170
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, was it actually non-trivial to find the -Zl documentation? It's available even in cl /?
output... I'd argue that if something needs to be added, then it would be more appropriate to discuss the subtlety of the -MT in the context. The symbol decoration part... Either way, I'd wait till maintainers make the choice which direction will be preferred, this, #717 or #725 ;-)
I worry that this solution may not address the core issue. Yes, omitting the CRT library name from a library will get rid of linker errors. But does this also hide potential conflicts that should be handled? For example, do the CRT (or even C++) headers care about static/dynamic or debug/no-debug? If they do then using |
As for static/dynamic, the only difference is when As for debug/non-debug. Well, it would only be a problem if a function prototype would change depending on Now an observation. Let's say you add
Well, doesn't it depend on definition of the core issue? :-) :-) :-) Formally speaking |
To my knowledge, rustc does not pass the You have outlined some ways in which C/C++ compilations changes depending on the options chosen, which on their own suggest |
It does not? But... How..................... Hmm... You're right!!! I must have mixed it up with windows-gnu. But again, how?! How does it work as if
It should be noted that
At the same time it's not formally reasonable to expect that arbitrary code would work without any adaptation. After all, it has to play on the host's, Rust's in this case, terms. And hence it's not unreasonable to limit the playfield. But I hear you! On the other hand, I'd say that the abovementioned swiping-under-the-rug is not exactly the definition of not-making-assumptions... |
Let's re-frame the problem description. Is |
Just in case for reference. To the question about "efficacy" of the .static_crt() method. It's perfectly possible to trigger linking errors by calling |
Since this crate doesn't actually make decisions about final application linking, it's only appropriate to not interfere with the process. This can be achieved by relying on default MSVC behaviour that allows linking with either VCCRT version, static or dynamic...
Complete make-over. Have a look, @ChrisDenton. |
Just in case, I'm not making this case. |
Other issues aside, I'm not convinced we should be changing the defaults here (see also the recent issues with changing asm defaults). Currently the import libraries Rust uses depends on The desire to have a more versatile way to override the defaults provided by cc-rs is something we should support but not at the expense of doing the wrong thing by default. |
Do you think it shouldn't? That sounds pretty reasonable behavior to me, and is consistent with how we set many other flags... |
Which is not actually happening. Attempt to mix makes the linker issue warnings (which rustc just swipes under the rug), but no actual mixing takes place. The ambiguity is resolved by prioritizing the command line over /defaultlib in the first object file with linker directive. Rustc passes the crt library on the command line which takes the priority, hence it's always in control. And it's only appropriate if cc-rs stays out of the way... |
Correct, I think it shouldn't. But it's just me :-) Just to rehash. The cc-rs's purpose is to produce .a files for rustc to link with. It's not its business to try to interfere with linking decisions. And there is a way to achieve the goal. I admit that omitting As for other flags. I don't know if you're talking about what I think you would, but there are a number of places where I feel the project fell for suboptimal solutions in a heat of a moment, so to say :-) Well, I'm just rambling... Cheers. |
With the current behaviour (if you don't override anything) cc-rs matches what rustc does. If you set crt-static in rustc then rustc with set I have a number of issues with the way Rust handles linking the C runtime and the UCRT but I do think cc-rs should match it as a default. |
Does it though? As already discussed, the difference between /MT and /MD is a) /defaultlib directives in .drective section, and b) |
If passing Using |
As already discussed,
Can you give a concrete example of one? As none can be observed on libc crate's boundary. And my assertion is that there are actually none. Again, the lack of the abovementioned symbol decoration is not an issue, it's dealt with, and libc crate depends on this fact. |
|
So you mean that you expect that application code somebody compiles with cc-rs could check for it. I've already addressed this hypothetical as "At the same time it's not formally reasonable to expect that arbitrary code would work without any adaptation. After all, it has to play on the host's, Rust's in this case, terms." In other words, if somebody checks for it and it turns to be a problem, it would be for them to resolve :-) |
Any code, including the C/C++ CRT headers themselves, can check defines, now or in the future. It would be unreasonable to expect everyone else to audit 30 years of C/C++ code and remove and fixup Yes, we can say people should adapt their C/C++ code to work with Rust tooling, but why should they when simply passing the right |
(I'd also note that people do also use |
And assertion is that all C CRT headers do is symbol decoration. But there you actually found stronger argument in your favour:-) Even a defeater:-) The C++ CRT. Trouble is that |
??? Where does cc-rs do that? Or are you referring to cdylib as crate type? In this case it's [still] rustc that calls the shorts at link time, and not a problem to link -MT -Zl C[!] into a .dll [in general or cdylib in particular]... |
Well, it should still be possible to use -MT -Zl with C files and follow Rust's crt-static when compiling C++ files... But it surely would be even more controversial :-) :-) :-) |
Since this crate doesn't actually make decisions about final application linking, it's only appropriate to not interfere with the process. This can be achieved
with -MT -Zl, because object modules compiled with these flags can be linkedby relying on default MSVC behaviour that allows linking with either VCCRT version, static or dynamic, debug or release...It's suggested to use -Zl when building Rust static library. For reference, -Zl doesn't affect linking directives set with in Windows headers to add references to not-so-common libraries like mfc.lib, windowscodecs.lib to name just a pair.
[Even an alternative solution to #717.]