Apple: Always pass SDK root when linking with `cc`, and pass it via `SDKROOT` env var
Fixes https://github.com/rust-lang/rust/issues/80817, fixes https://github.com/rust-lang/rust/issues/96943, and generally simplifies our linker invocation on Apple platforms.
Part of https://github.com/rust-lang/rust/issues/129432.
### Necessary background on trampoline binaries
The developer binaries such as `/usr/bin/cc` and `/usr/bin/clang` are actually trampolines (similar in spirit to the Rust binaries in `~/.cargo/bin`) which effectively invokes `xcrun` to get the current Xcode developer directory, which allows it to find the actual binary under `/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/*`.
This binary is then launched with the following environment variables set (but none of them are set if `SDKROOT` is set explicitly):
- `SDKROOT=/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk`
- `LIBRARY_PATH=/usr/local/lib` (appended)
- `CPATH=/usr/local/include` (appended)
- `MANPATH=/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/share/man:/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/usr/share/man:/Applications/Xcode.app/Contents/Developer/usr/share/man:/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/share/man:` (prepended)
This allows the user to type e.g. `clang foo.c` in their terminal on macOS, and have it automatically pick up a suitable Clang binary and SDK from either an installed Xcode.app or the Xcode Command Line Tools.
(It acts roughly as-if you typed `xcrun -sdk macosx clang foo.c`).
### Finding a suitable SDK
All compilation on macOS is cross-compilation using SDKs, there are no system headers any more (`/usr/include` is gone), and the system libraries are elsewhere in the file system (`/usr/lib` is basically empty). Instead, the logic for finding the SDK is handled by the `/usr/bin/cc` trampoline (see above).
But relying on the `cc` trampoline doesn't work when:
- Cross-compiling, since a different SDK is needed there.
- Invoking the linker directly, since the linker doesn't understand `SDKROOT`.
- Linking build scripts inside Xcode (see https://github.com/rust-lang/rust/issues/80817), since Xcode prepends `/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin` to `PATH`, which means `cc` refers to the _actual_ Clang binary, and we end up with the wrong SDK root specified.
Basically, we cannot rely on the trampoline at all, so the last commit removes the special-casing that was done when linking with `cc` for macOS (i.e. the most common path), so that **we now always invoke `xcrun` (if `SDKROOT` is not explicitly specified) to find the SDK root**.
Making sure this is non-breaking has a few difficulties though, namely that the user might not have Xcode installed, and that the compiler driver may not understand the `-isysroot` flag. These difficulties are explored below.
#### No Xcode
There are several compiler drivers which work without Xcode by bundling their own SDK, including `zig cc`, Nixpkgs' `clang` and Homebrew's `llvm` package. Additionally, `xcrun` is rarely available when cross-compiling from non-macOS and instead the user might provide a downloaded SDK manually with `-Clink-args=...`.
We do still want to _try_ to invoke `xcrun` if possible, since it is usually the SDK that the user wants (and if not, the environment should override `xcrun`, such as is done by Nixpkgs). But we do not want failure to invoke `xcrun` to stop the linking process. This is changed in the second-to-last commit.
#### `SDKROOT` vs. `-isysroot`
The exact reasoning why we do not always pass the SDK root when linking on macOS eludes me (the git history dead ends in rust-lang/rust#100286), but I suspect it's because we want to support compiler drivers which do not support the `-isysroot` option.
To make sure that such use-cases continue to work, we now pass the SDK root via the `SDKROOT` environment variable. This way, compiler drivers that support setting the SDK root (such as Clang and GCC) can use it, while compiler drivers that don't (presumably because they figure out the SDK in some other way) can just ignore it.
One small danger here would be if there's some compiler driver out there which works with the `-isysroot` flag, but not with the `SDKROOT` environment variable. I am not aware of any?
In a sense, this also shifts the blame; if a compiler driver does not understand `SDKROOT`, it won't work with e.g. `xcrun -sdk macosx15.0 $tool` either, so it can more clearly be argued that this is incorrect behaviour on the part of the tool.
Note also that this overrides the behaviour discussed above (`/usr/bin/cc` sets some extra environment variables), I will argue that is fine since `MANPATH` and `CPATH` is useless when linking, and `/usr/local/lib` is empty on a default system at least since macOS 10.14 (it might be filled by extra libraries installed by the user, but I'll argue that if we want it to be part of the default library search path, we should set it explicitly so that it's also set when linking with `-Clinker=ld`).
### Considered alternatives
- Invoke `/usr/bin/cc` instead of `cc`.
- This breaks many other use-cases though where overriding `cc` in the PATH is desired.
- Look up `which cc`, and do special logic if in Xcode toolchain.
- Seems brittle, and besides, it's not the `cc` in the Xcode toolchain that's wrong, it's the `/usr/bin/cc` behaviour that is a bit too magical.
- Invoke `xcrun --sdk macosx cc`.
- This completely ignores `SDKROOT`, so we'd still have to parse that first to figure out if it's suitable or not, but would probably be workable.
- Maybe somehow configure the linker with extra flags such that it'll be able to link regardless of linking for macOS or e.g. iOS? Though I doubt this is possible.
- Bundle the SDK, similar to `zig-cc`.
- Comes with it's own host of problems.
### Testing
Tested that this works with the following `-Clinker=...`:
- [x] Default (`cc`)
- [x] `/usr/bin/ld`
- [x] Actual Clang from Xcode (`/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/clang`)
- [x] `/usr/bin/clang` (invoked via `clang` instead of `cc`)
- [x] Homebrew's `llvm` package (ignores `SDKROOT`, uses their own SDK)
- [x] Homebrew's `gcc` package (`SDKROOT` is preferred over their own SDK)
- [x] ~Macports `clang`~ Couldn't get it to build
- [x] Macports `gcc` (`SDKROOT` is preferred over their own SDK)
- [x] Zig CC installed via. homebrew (ignores both `-isysroot` and `SDKROOT`, uses their own SDK)
- [x] Nixpkgs `clang` (ignores `SDKROOT`, uses their own SDK)
- [x] Nixpkgs `gcc` (ignores `SDKROOT`, uses their own SDK)
- [x] ~[`cosmocc`](https://github.com/jart/cosmopolitan)?~ Doesn't accept common flags (like `-arch`)
CC ```````@BlackHoleFox``````` ```````@thomcc```````
search graph: improve rebasing and add forced ambiguity support
Based on rust-lang/rust#142774
This slightly strengthens rebasing and actually checks for the property we want to maintain. There are two additional optimizations we can and should do here:
- we should be able to just always rebase if cycle heads already have a provisional result from a previous iteration
- we currently only apply provisional cache entries if the `path_to_entry` matches exactly. We should be able to extend this e.g. if you have an entry for `B` in `ABA` where the path `BA` is coinductive, then we can use this entry even if the current path from `A` to `B` is inductive.
---
I've also added support for `PathKind::ForcedAmbiguity` which always forced the initial provisional result to be ambiguous. A am using this for cycles involving negative reasons, which is currently only used by the fuzzer in https://github.com/lcnr/search_graph_fuzz. Consider the following setup: A goal `A` which only holds if `B` does not hold, and `B` which only holds if `A` does not hold.
- A only holds if B does not hold, results in X
- B only holds if A does not hold, results in !X
- A cycle, provisional result X
- B only holds if A does not hold, results in X
- A only holds if B does not hold, results in !X
- B cycle, provisional result X
With negative reasoning, the result of cycle participants depends on their position in the cycle. This means using cache entries while other entries are on the stack/have been popped is wrong. It's also generally just kinda iffy. By always forcing the initial provisional result of such cycles to be ambiguity, we can avoid this, as "not maybe" is just "maybe" again.
Rust kind of has negative reasoning due to incompleteness, consider the following setup:
- `T::Foo eq u32`
- normalize `T::Foo`
- via impl -> `u32`
- via param_env -> `T`
- nested goals...
`T::Foo eq u32` holds exactly if the nested goals of the `param_env` candidate do not hold, as preferring that candidate over the impl causes the alias-relate to fail. This means the current provisional cache may cause us to ignore `param_env` preference in rare cases. This is not unsound and I don't care about it, as we already have this behavior when rerunning on changed fixpoint results:
- `T: Trait`
- via impl ok
- via env
- `T: Trait` non-productive cycle
- result OK, rerun changed provisional result
- `T: Trait`
- via impl ok
- via env
- `T: Trait` using the provisional result, can be thought of as recursively expanding the proof tree
- via impl ok
- via env <don't care>
- prefer the env candidate, reached fixpoint
---
One could imaging changing `ParamEnv` candidates or the impl shadowing check to use `PathKind::ForcedAmbiguity` to make the search graph less observable instead of only using it for fuzzing. However, incomplete candidate preference isn't really negative reasoning and doing this is a breaking change https://github.com/rust-lang/trait-system-refactor-initiative/issues/114
r? `@compiler-errors`
The exact reasoning why we do not always pass the SDK root when linking
on macOS eludes me, but I suspect it's because we want to support
compiler drivers which do not support the `-isysroot` option.
Since we now pass the SDK root via the environment variable SDKROOT,
compiler drivers that don't support it can just ignore it.
Similarly, since we only warn when xcrun fails, users that expect their
compiler driver to provide the SDK location can do so now.
This is more in-line with what Apple's tooling expects, and allows us to
better support custom compiler drivers (such as certain Homebrew and
Nixpkgs compilers) that prefer their own `-isysroot` flag.
Effectively, we now invoke the compiler driver as-if it was invoked as
`xcrun -sdk $sdk_name $tool`.
Modify `AttributeTemplate` to support list of alternatives for list and name value attribute styles.
Suggestions now provide more correct suggested code:
```
error[E0805]: malformed `used` attribute input
--> $DIR/used_with_multi_args.rs:3:1
|
LL | #[used(compiler, linker)]
| ^^^^^^------------------^
| |
| expected a single argument here
|
help: try changing it to one of the following valid forms of the attribute
|
LL - #[used(compiler, linker)]
LL + #[used(compiler)]
|
LL - #[used(compiler, linker)]
LL + #[used(linker)]
|
LL - #[used(compiler, linker)]
LL + #[used]
|
```
instead of the prior "masking" of the lack of this feature by suggesting pipe-separated lists:
```
error[E0805]: malformed `used` attribute input
--> $DIR/used_with_multi_args.rs:3:1
|
LL | #[used(compiler, linker)]
| ^^^^^^------------------^
| |
| expected a single argument here
|
help: try changing it to one of the following valid forms of the attribute
|
LL - #[used(compiler, linker)]
LL + #[used(compiler|linker)]
|
LL - #[used(compiler, linker)]
LL + #[used]
|
```
Improve suggestion for "missing function argument" on multiline call
`rustc` has a very neat suggestion when the argument count does not match, with a nice placeholder that shows where an argument may be missing. Unfortunately the suggestion is always single-line, even when the function call spans across multiple lines. With this PR, `rustc` tries to guess if the function call is multiline or not, and emits a multiline suggestion when required.
r? `@jdonszelmann`
Set the dead_on_return attribute (added in LLVM 21) for arguments
that are passed indirectly, but not byval.
This indicates that the value of the argument on return does not
matter, enabling additional dead store elimination.
Ignore coroutine witness type region args in auto trait confirmation
## The problem
Consider code like:
```
async fn process<'a>() {
Box::pin(process()).await;
}
fn require_send(_: impl Send) {}
fn main() {
require_send(process());
}
```
When proving that the coroutine `{coroutine@process}::<'?0>: Send`, we end up instantiating a nested goal `{witness@process}::<'?0>: Send` by synthesizing a witness type from the coroutine's args:
Proving a coroutine witness type implements an auto trait requires looking up the coroutine's witness types. The witness types are a binder that look like `for<'r> { Pin<Box<{coroutine@process}::<'r>>> }`. We instantiate this binder with placeholders and prove `Send` on the witness types. This ends up eventually needing to prove something like `{coroutine@process}::<'!1>: Send`. Repeat this process, and we end up in an overflow during fulfillment, since fulfillment does not use freshening.
This can be visualized with a trait stack that ends up looking like:
* `{coroutine@process}::<'?0>: Send`
* `{witness@process}::<'?0>: Send`
* `Pin<Box<{coroutine@process}::<'!1>>>: Send`
* `{coroutine@process}::<'!1>: Send`
* ...
* `{coroutine@process}::<'!2>: Send`
* `{witness@process}::<'!2>: Send`
* ...
* overflow!
The problem here specifically comes from the first step: synthesizing a witness type from the coroutine's args.
## Why wasn't this an issue before?
Specifically, before 63f6845e57, this wasn't an issue because we were instead extracting the witness from the coroutine type itself. It turns out that given some `{coroutine@process}::<'?0>`, the witness type was actually something like `{witness@process}::<'erased>`!
So why do we end up with a witness type with `'erased` in its args? This is due to the fact that opaque type inference erases all regions from the witness. This is actually explicitly part of opaque type inference -- changing this to actually visit the witness types actually replicates this overflow even with 63f6845e57 reverted:
ca77504943/compiler/rustc_borrowck/src/type_check/opaque_types.rs (L303-L313)
To better understand this difference and how it avoids a cycle, if you look at the trait stack before 63f6845e57, we end up with something like:
* `{coroutine@process}::<'?0>: Send`
* `{witness@process}::<'erased>: Send` **<-- THIS CHANGED**
* `Pin<Box<{coroutine@process}::<'!1>>>: Send`
* `{coroutine@process}::<'!1>: Send`
* ...
* `{coroutine@process}::<'erased>: Send` **<-- THIS CHANGED**
* `{witness@process}::<'erased>: Send` **<-- THIS CHANGED**
* coinductive cycle! 🎉
## So what's the fix?
This hack replicates the behavior in opaque type inference to erase regions from the witness type, but instead erasing the regions during auto trait confirmation. This is kinda a hack, but is sound. It does not need to be replicated in the new trait solver, of course.
---
I hope this explanation makes sense.
We could beta backport this instead of the revert https://github.com/rust-lang/rust/pull/145193, but then I'd like to un-revert that on master in this PR along with landing this this hack. Thoughts?
r? lcnr
`NllRegionVariableOrigin` remove `from_forall`
See added comment in the only place it was used.
cc rust-lang/rust#144988 `@amandasystems,` going to merge that PR first.
Check coroutine upvars in dtorck constraint
Fixrust-lang/rust#144155.
This PR fixes an unsoundness where we were not considering coroutine upvars as drop-live if the coroutine interior types (witness types) had nothing which required drop.
In the case that the coroutine does not have any interior types that need to be dropped, then we don't need to treat all of the upvars as use-live; instead, this PR uses the same logic as closures, and descends into the upvar types to collect anything that must be drop-live. The rest of this PR is reworking the comment to explain the behavior here.
r? `@lcnr` or reassign 😸
---
Just some thoughts --- a proper fix for this whole situation would be to consider `TypingMode` in the `needs_drop` function, and just calling `coroutine_ty.needs_drop(tcx, typing_env)` in the dtorck constraint check.
During MIR building, we should probably use a typing mode that stalls the local coroutines and considers them to be unconditionally drop, or perhaps just stall *all* coroutines in analysis mode. Then in borrowck mode, we can re-check `needs_drop` but descend into witness types properly. https://github.com/rust-lang/rust/pull/144158 implements this experimentally.
This is a pretty involved fix, and conflicts with some in-flight changes (rust-lang/rust#144157) that I have around removing coroutine witnesses altogether. I'm happy to add a FIXME to rework this whole approach, but I don't want to block this quick fix since it's obviously more correct than the status-quo.
Reject relaxed bounds inside associated type bounds (ATB)
**Reject** relaxed bounds — most notably `?Sized` — inside associated type bounds `TraitRef<AssocTy: …>`.
This was previously accepted without warning despite being incorrect: ATBs are *not* a place where we perform *sized elaboration*, meaning `TraitRef<AssocTy: …>` does *not* elaborate to `TraitRef<AssocTy: Sized + …>` if `…` doesn't contain `?Sized`. Therefore `?Sized` is meaningless. In no other (stable) place do we (intentionally) allow relaxed bounds where we don't also perform sized elab, this is highly inconsistent and confusing! Another point of comparison: For the desugared `$SelfTy: TraitRef, $SelfTy::AssocTy: …` we don't do sized elab either (and thus also don't allow relaxed bounds).
Moreover — as I've alluded to back in https://github.com/rust-lang/rust/pull/135841#pullrequestreview-2619462717 — some later validation steps only happen during sized elaboration during HIR ty lowering[^1]. Namely, rejecting duplicates (e.g., `?Trait + ?Trait`) and ensuring that `Trait` in `?Trait` is equal to `Sized`[^2]. As you can probably guess, on stable/master we don't run these checks for ATBs (so we allow even more nonsensical bounds like `Iterator<Item: ?Copy>` despite T-types's ruling established in the FCP'ed rust-lang/rust#135841).
This PR rectifies all of this. I cratered this back in 2025-01-10 with (allegedly) no regressions found ([report](https://github.com/rust-lang/rust/pull/135331#issuecomment-2585330783), [its analysis](https://github.com/rust-lang/rust/pull/135331#issuecomment-2585356422)). [However a contributor manually found two occurrences](https://github.com/rust-lang/rust/issues/135229#issuecomment-2581832852) of `TraitRef<AssocTy: ?Sized>` in small hobby projects (presumably via GH code search). I immediately sent downstream PRs: https://github.com/Gui-Yom/turbo-metrics/pull/14, https://github.com/ireina7/summon/pull/1 (however, the owners have showed no reaction so far).
I'm leaning towards banning these forms **without a FCW** because a FCW isn't worth the maintenance cost[^3]. Note that associated type bounds were stabilized in 1.79.0 (released 2024-06-13 which is 13 months ago), so the proliferation of ATBs shouldn't be that high yet. If you think we should do another crater run since the last one was 6 months ago, I'm fine with that.
Fixesrust-lang/rust#135229.
[^1]: I consider this a flaw in the implementation and [I've already added a huge FIXME](82a02aefe0/compiler/rustc_hir_analysis/src/hir_ty_lowering/bounds.rs (L195-L207)).
[^2]: To be more precise, if the internal flag `-Zexperimental-default-bounds` is provided other "default traits" (needs internal feature `lang_items`) are permitted as well (cc closely related internal feature: `more_maybe_bounds`).
[^3]: Having to track this and adding an entire lint whose remnants would remain in the code base forever (we never *fully* remove lints).
Tweak spans providing type context on errors when involving macros
Do not point at macro invocation multiple times when we try to add span labels mentioning what type each expression has, which is unnecessary when the error is at a macro invocation.
Point at the `Fn()` or `FnMut()` bound that coerced a closure, which caused a move error
When encountering a move error involving a closure because the captured value isn't `Copy`, and the obligation comes from a bound on a type parameter that requires `Fn` or `FnMut`, we point at it and explain that an `FnOnce` wouldn't cause the move error.
```
error[E0507]: cannot move out of `foo`, a captured variable in an `Fn` closure
--> f111.rs:15:25
|
14 | fn do_stuff(foo: Option<Foo>) {
| --- ----------- move occurs because `foo` has type `Option<Foo>`, which does not implement the `Copy` trait
| |
| captured outer variable
15 | require_fn_trait(|| async {
| -- ^^^^^ `foo` is moved here
| |
| captured by this `Fn` closure
16 | if foo.map_or(false, |f| f.foo()) {
| --- variable moved due to use in coroutine
|
help: `Fn` and `FnMut` closures require captured values to be able to be consumed multiple times, but an `FnOnce` consume them only once
--> f111.rs:12:53
|
12 | fn require_fn_trait<F: Future<Output = ()>>(_: impl Fn() -> F) {}
| ^^^^^^^^^
help: consider cloning the value if the performance cost is acceptable
|
16 | if foo.clone().map_or(false, |f| f.foo()) {
| ++++++++
```
Fixrust-lang/rust#68119, by pointing at `Fn` and `FnMut` bounds involved in move errors.
Detect struct construction with private field in field with default
When trying to construct a struct that has a public field of a private type, suggest using `..` if that field has a default value.
```
error[E0603]: struct `Priv1` is private
--> $DIR/non-exhaustive-ctor-2.rs:19:39
|
LL | let _ = S { field: (), field1: m::Priv1 {} };
| ------ ^^^^^ private struct
| |
| while setting this field
|
note: the struct `Priv1` is defined here
--> $DIR/non-exhaustive-ctor-2.rs:14:4
|
LL | struct Priv1 {}
| ^^^^^^^^^^^^
help: the type `Priv1` of field `field1` is private, but you can construct the default value defined for it in `S` using `..` in the struct initializer expression
|
LL | let _ = S { field: (), .. };
| ~~
```
When operating on expressions, `cfg_select!` can now handle expressions
without braces. (It still requires braces for other things, such as
items.)
Expand the test coverage and documentation accordingly.
Do not point at macro invocation which expands to an inference error. Avoid the following:
```
error[E0308]: mismatched types
--> $DIR/does-not-have-iter-interpolated.rs:12:5
|
LL | quote!($($nonrep)*);
| ^^^^^^^^^^^^^^^^^^^
| |
| expected `HasIterator`, found `ThereIsNoIteratorInRepetition`
| expected due to this
| here the type of `has_iter` is inferred to be `ThereIsNoIteratorInRepetition`
```
Fix typo with paren rustc_llvm/build.rs
The current parenthesis looks suspect: it means that OpenHarmony is always excluded whereas it looks like it was intended to only be excluded if the architecture was Arm.
Since Rust doesn't support the other architectures with OpenHarmony, there currently isn't a bug but this cleans up some suspicious code and avoids a potential future annoyance for someone trying to bring up a new triple.
r? `@Amanieu`
Use `eq_ignore_ascii_case` to avoid heap alloc in `detect_confuse_type`
A small optimization has been made, using `to_ascii_lowercase()` instead of `to_lowercase().to_string()`.
r? compiler
