Fix error when building rustc with a custom libc

[IsaacWoods]

When pulling a custom libc into a rustc build (with the rustc-dep-of-std feature set), the following error occurs:

error: unused attribute
  --> /hdd/libc/src/lib.rs:29:1
   |
29 | #![no_std]
   | ^^^^^^^^^^
   |
   = note: `-D unused-attributes` implied by `-D warnings`

error: crate-level attribute should be in the root module
  --> /hdd/libc/src/lib.rs:29:1
   |
29 | #![no_std]
   | ^^^^^^^^^^

I think this is because both the no_std and no_core attributes are specified, although the error message doesn't make this very clear. This PR changes this so no_std is only supplied when the rustc-dep-of-std feature is not.

[rust-highfive]

r? @JohnTitor

(rust-highfive has picked a reviewer for you, use r? to override)

[JohnTitor]

AFAIK this is necessary as mentioned in #1268 and rust-lang/rust#73096 (comment).
Also, we don't provide any support for rustc-dep-of-std use on external.

[IsaacWoods]

I'm not sure I understand - is libc special-cased when it's built as part of the compiler (further than applying rustc-dep-of-std? Why does that special casing not happen when the crate is still called libc, but is specified by path rather than version?

I'm sure there are good reasons, but why does no_core not behaving properly mean that libc is special-cased to be allowed to have both, rather than fixing that in the compiler? Also, this change works and builds a working compiler - are we sure that the root cause of this situation is still present?

Also, we don't provide any support for rustc-dep-of-std use on external.

This position requires anyone testing changes to libc from rustc to make this change locally, and effectively precludes out-of-tree targets from being able to use their own libc version without making this change anyway and hoping they don't break.

[JohnTitor]

I'm not sure I understand - is libc special-cased when it's built as part of the compiler (further than applying rustc-dep-of-std? Why does that special casing not happen when the crate is still called libc, but is specified by path rather than version?

I'm sure there are good reasons, but why does no_core not behaving properly mean that libc is special-cased to be allowed to have both, rather than fixing that in the compiler? Also, this change works and builds a working compiler - are we sure that the root cause of this situation is still present?

Maybe bootstrap step or something would be related but I couldn't find any reference at all.
@alexcrichton Do you know anything about this?

Also, we don't provide any support for rustc-dep-of-std use on external.

This position requires anyone testing changes to libc from rustc to make this change locally, and effectively precludes out-of-tree targets from being able to use their own libc version without making this change anyway and hoping they don't break.

I should've clarified that we didn't generally. The important thing here is not to break rust-lang/rust. So, if it won't break the build on rust-lang/rust and makes much sense, I'm fine to accept it.

[alexcrichton]

No the feature is the only special-casing in rust-lang/rust. Cargo squashes warnings from all non-path dependencies, though, so when it's built from crates.io you don't see warnings. If you're using a path dependency or [patch] then you'll see warnings.

FWIW this seems reasonable, I don't think this generated a warning when it was first implemented, but keeping warnings out is typically nice.

[IsaacWoods]

No the feature is the only special-casing in rust-lang/rust. Cargo squashes warnings from all non-path dependencies, though, so when it's built from crates.io you don't see warnings. If you're using a path dependency or [patch] then you'll see warnings.

Ah, this helps my understanding a lot, thank you! I got the impression that passing #![no_std] unconditionally was required for correctness for libc for a bizarre reason, but it just doesn't hit this error in a normal rustc build because the warning is suppressed if I understand correctly.

I should've clarified that we didn't generally. The important thing here is not to break rust-lang/rust. So, if it won't break the build on rust-lang/rust and makes much sense, I'm fine to accept it.

Ah totally! I've read back what I wrote and it seems a little combative in hindsight - sorry for that!

[JohnTitor]

Thanks Alex for pointing it out!
Then I'm happy to r+ this :)

[JohnTitor]

@bors r+

[bors]

📌 Commit 48c4482 has been approved by JohnTitor

[bors]

⌛ Testing commit 48c4482 with merge bb8fe9a...

[bors]

☀️ Test successful - checks-actions, checks-cirrus-freebsd-11, checks-cirrus-freebsd-12, checks-cirrus-freebsd-13
Approved by: JohnTitor
Pushing bb8fe9a to master...