Commit Graph

6307 Commits

Author SHA1 Message Date
Mads Marquart
de212963f8 Relax framework linking test
This test was introduced in #118644, but was over-specified in that it assumed the path of the linker was always `cc`.
2024-04-06 09:00:07 +02:00
bors
11853ecd86 Auto merge of #123517 - GuillaumeGomez:rollup-eys3jfp, r=GuillaumeGomez
Rollup of 8 pull requests

Successful merges:

 - #121419 (Add aarch64-apple-visionos and aarch64-apple-visionos-sim tier 3 targets)
 - #123159 (Fix target-cpu fpu features on Arm R/M-profile)
 - #123487 (CFI: Restore typeid_for_instance default behavior)
 - #123500 (Revert removing miri jobserver workaround)
 - #123505 (Revert "Use OS thread name by default")
 - #123509 (Add jieyouxu to compiler review rotation and as a reviewer for `tests/run-make`, `src/tools/run-make-support` and `src/tools/compiletest`)
 - #123514 (Fix typo in `compiler/rustc_middle/src/traits/solve/inspect.rs`)
 - #123515 (Use `include` command to reduce code duplication)

r? `@ghost`
`@rustbot` modify labels: rollup
2024-04-05 22:12:43 +00:00
Guillaume Gomez
74a5bc6c9e
Rollup merge of #121419 - agg23:xrOS-pr, r=davidtwco
Add aarch64-apple-visionos and aarch64-apple-visionos-sim tier 3 targets

Introduces `aarch64-apple-visionos` and `aarch64-apple-visionos-sim` as tier 3 targets. This allows native development for the Apple Vision Pro's visionOS platform.

This work has been tracked in https://github.com/rust-lang/compiler-team/issues/642. There is a corresponding `libc` change https://github.com/rust-lang/libc/pull/3568 that is not required for merge.

Ideally we would be able to incorporate [this change](https://github.com/gimli-rs/object/pull/626) to the `object` crate, but the author has stated that a release will not be cut for quite a while. Therefore, the two locations that would reference the xrOS constant from `object` are hardcoded to their MachO values of 11 and 12, accompanied by TODOs to mark the code as needing change. I am open to suggestions on what to do here to get this checked in.

# Tier 3 Target Policy

At this tier, the Rust project provides no official support for a target, so we place minimal requirements on the introduction of targets.

> A tier 3 target must have a designated developer or developers (the "target maintainers") on record to be CCed when issues arise regarding the target. (The mechanism to track and CC such developers may evolve over time.)

See [src/doc/rustc/src/platform-support/apple-visionos.md](e88379034a/src/doc/rustc/src/platform-support/apple-visionos.md)

> Targets must use naming consistent with any existing targets; for instance, a target for the same CPU or OS as an existing Rust target should use the same name for that CPU or OS. Targets should normally use the same names and naming conventions as used elsewhere in the broader ecosystem beyond Rust (such as in other toolchains), unless they have a very good reason to diverge. Changing the name of a target can be highly disruptive, especially once the target reaches a higher tier, so getting the name right is important even for a tier 3 target.
> * Target names should not introduce undue confusion or ambiguity unless absolutely necessary to maintain ecosystem compatibility. For example, if the name of the target makes people extremely likely to form incorrect beliefs about what it targets, the name should be changed or augmented to disambiguate it.
> * If possible, use only letters, numbers, dashes and underscores for the name. Periods (.) are known to cause issues in Cargo.

This naming scheme matches `$ARCH-$VENDOR-$OS-$ABI` which is matches the iOS Apple Silicon simulator (`aarch64-apple-ios-sim`) and other Apple targets.

> Tier 3 targets may have unusual requirements to build or use, but must not
  create legal issues or impose onerous legal terms for the Rust project or for
  Rust developers or users.
>  - The target must not introduce license incompatibilities.
>  - Anything added to the Rust repository must be under the standard Rust license (`MIT OR Apache-2.0`).
>  - The target must not cause the Rust tools or libraries built for any other host (even when supporting cross-compilation to the target) to depend on any new dependency less permissive than the Rust licensing policy. This applies whether the dependency is a Rust crate that would require adding new license exceptions (as specified by the `tidy` tool in the rust-lang/rust repository), or whether the dependency is a native library or binary. In other words, the introduction of the target must not cause a user installing or running a version of Rust or the Rust tools to besubject to any new license requirements.
>  - Compiling, linking, and emitting functional binaries, libraries, or other code for the target (whether hosted on the target itself or cross-compiling from another target) must not depend on proprietary (non-FOSS) libraries. Host tools built for the target itself may depend on the ordinary runtime libraries supplied by the platform and commonly used by other applications built for the target, but those libraries must not be required for code generation for the target; cross-compilation to the target must not require such libraries at all. For instance, `rustc` built for the target may depend on a common proprietary C runtime library or console output library, but must not depend on a proprietary code generation library or code optimization library. Rust's license permits such combinations, but the Rust project has no interest in maintaining such combinations within the scope of Rust itself, even at tier 3.
> - "onerous" here is an intentionally subjective term. At a minimum, "onerous" legal/licensing terms include but are *not* limited to: non-disclosure requirements, non-compete requirements, contributor license agreements (CLAs) or equivalent, "non-commercial"/"research-only"/etc terms, requirements conditional on the employer or employment of any particular Rust developers, revocable terms, any requirements that create liability for the Rust project or its developers or users, or any requirements that adversely affect the livelihood or prospects of the Rust project or its developers or users.

This contribution is fully available under the standard Rust license with no additional legal restrictions whatsoever. This PR does not introduce any new dependency less permissive than the Rust license policy.

The new targets do not depend on proprietary libraries.

> Tier 3 targets should attempt to implement as much of the standard libraries as possible and appropriate (core for most targets, alloc for targets that can support dynamic memory allocation, std for targets with an operating system or equivalent layer of system-provided functionality), but may leave some code unimplemented (either unavailable or stubbed out as appropriate), whether because the target makes it impossible to implement or challenging to implement. The authors of pull requests are not obligated to avoid calling any portions of the standard library on the basis of a tier 3 target not implementing those portions.

This new target mirrors the standard library for watchOS and iOS, with minor divergences.

> The target must provide documentation for the Rust community explaining how to build for the target, using cross-compilation if possible. If the target supports running binaries, or running tests (even if they do not pass), the documentation must explain how to run such binaries or tests for the target, using emulation if possible or dedicated hardware if necessary.

Documentation is provided in [src/doc/rustc/src/platform-support/apple-visionos.md](e88379034a/src/doc/rustc/src/platform-support/apple-visionos.md)

> Neither this policy nor any decisions made regarding targets shall create any binding agreement or estoppel by any party. If any member of an approving Rust team serves as one of the maintainers of a target, or has any legal or employment requirement (explicit or implicit) that might affect their decisions regarding a target, they must recuse themselves from any approval decisions regarding the target's tier status, though they may otherwise participate in discussions.
> * This requirement does not prevent part or all of this policy from being cited in an explicit contract or work agreement (e.g. to implement or maintain support for a target). This requirement exists to ensure that a developer or team responsible for reviewing and approving a target does not face any legal threats or obligations that would prevent them from freely exercising their judgment in such approval, even if such judgment involves subjective matters or goes beyond the letter of these requirements.

> Tier 3 targets must not impose burden on the authors of pull requests, or other developers in the community, to maintain the target. In particular, do not post comments (automated or manual) on a PR that derail or suggest a block on the PR based on a tier 3 target. Do not send automated messages or notifications (via any medium, including via `@)` to a PR author or others involved with a PR regarding a tier 3 target, unless they have opted into such messages.
> * Backlinks such as those generated by the issue/PR tracker when linking to an issue or PR are not considered a violation of this policy, within reason. However, such messages (even on a separate repository) must not generate notifications to anyone involved with a PR who has not requested such notifications.

> Patches adding or updating tier 3 targets must not break any existing tier 2 or tier 1 target, and must not knowingly break another tier 3 target without approval of either the compiler team or the maintainers of the other tier 3 target.
> * In particular, this may come up when working on closely related targets, such as variations of the same architecture with different features. Avoid introducing unconditional uses of features that another variation of the target may not have; use conditional compilation or runtime detection, as appropriate, to let each target run code supported by that target.

I acknowledge these requirements and intend to ensure that they are met.

This target does not touch any existing tier 2 or tier 1 targets and should not break any other targets.
2024-04-05 22:33:25 +02:00
bors
9d79cd5f79 Auto merge of #122747 - Urgau:non-local-defs_perfect_impl, r=lcnr
Implement T-types suggested logic for perfect non-local impl detection

This implement [T-types suggested logic](https://github.com/rust-lang/rust/issues/121621#issuecomment-1976826895) for perfect non-local impl detection:

> for each impl, instantiate all local types with inference vars and then assemble candidates for that goal, if there are more than 1 (non-private impls), it does not leak

This extension to the current logic is meant to address issues reported in https://github.com/rust-lang/rust/issues/121621.

This PR also re-enables the lint `non_local_definitions` to warn-by-default.

Implementation was discussed in this [zulip thread](https://rust-lang.zulipchat.com/#narrow/stream/144729-t-types/topic/Implementing.20new.20non-local.20impl.20defs.20logic).

Fixes https://github.com/rust-lang/rust/issues/121621
Fixes https://github.com/rust-lang/rust/issues/121746

r? `@lcnr` *(feel free to re-roll)*
2024-04-05 20:09:57 +00:00
Urgau
2f2d5cc38d Put non_local_definitions lint back to warn-by-default 2024-04-05 19:25:58 +02:00
Urgau
8edf2558d2 Update non-local impl definition lint rule note 2024-04-05 19:25:58 +02:00
Urgau
a1d7bff7ef Eliminate false-positives in the non-local lint with the type-system 2024-04-05 19:25:43 +02:00
Guillaume Gomez
02ee8a8cee
Rollup merge of #123350 - compiler-errors:async-closure-by-move, r=oli-obk
Actually use the inferred `ClosureKind` from signature inference in coroutine-closures

A follow-up to https://github.com/rust-lang/rust/pull/123349, which fixes another subtle bug: We were not taking into account the async closure kind we infer during closure signature inference.

When I pass a closure directly to an arg like `fn(x: impl async FnOnce())`, that should have the side-effect of artificially restricting the kind of the async closure to `ClosureKind::FnOnce`. We weren't doing this -- that's a quick fix; however, it uncovers a second, more subtle bug with the way that `move`, async closures, and `FnOnce` interact.

Specifically, when we have an async closure like:
```
let x = Struct;
let c = infer_as_fnonce(async move || {
  println!("{x:?}");
}
```

The outer closure captures `x` by move, but the inner coroutine still immutably borrows `x` from the outer closure. Since we've forced the closure to by `async FnOnce()`, we can't actually *do* a self borrow, since the signature of `AsyncFnOnce::call_once` doesn't have a borrowed lifetime. This means that all `async move` closures that are constrained to `FnOnce` will fail borrowck.

We can fix that by detecting this case specifically, and making the *inner* async closure `move` as well. This is always beneficial to closure analysis, since if we have an `async FnOnce()` that's `move`, there's no reason to ever borrow anything, so `move` isn't artificially restrictive.
2024-04-05 16:38:51 +02:00
Guillaume Gomez
f2f8d8b722
Rollup merge of #123311 - Jules-Bertholet:andpat-everywhere, r=Nadrieril
Match ergonomics: implement "`&`pat everywhere"

Implements the eat-two-layers (feature gate `and_pat_everywhere`, all editions) ~and the eat-one-layer (feature gate `and_eat_one_layer_2024`, edition 2024 only, takes priority on that edition when both feature gates are active)~ (EDIT: will be done in later PR) semantics.

cc #123076

r? ``@Nadrieril``

``@rustbot`` label A-patterns A-edition-2024
2024-04-05 16:38:50 +02:00
Guillaume Gomez
cb6a1c8d45
Rollup merge of #122894 - compiler-errors:downgrade, r=lcnr
Move check for error in impl header outside of reporting

Fixes #121006

r? lcnr

test location kinda sucks, can move it if needed
2024-04-05 16:38:49 +02:00
bors
c0ddaef075 Auto merge of #123444 - saethlin:const-eval-inline-cycles, r=tmiasko
Teach MIR inliner query cycle avoidance about const_eval_select

Fixes https://github.com/rust-lang/rust/issues/122659

r? tmiasko
2024-04-05 04:34:05 +00:00
Jacob Pratt
e8b0c30578
Rollup merge of #123478 - maurer:cfi-call-once-addr-taken, r=compiler-errors
CFI: Add test for `call_once` addr taken

One of the proposed ways to reduce the non-passed argument erasure would cause this test to fail. Adding this now ensures that any attempt to reduce non-passed argument erasure won't make the same mistake.

r? `@compiler-errors`

cc `@rcvalle`
2024-04-04 21:16:59 -04:00
Jacob Pratt
fcb0e9d07a
Rollup merge of #123363 - lcnr:normalizes-to-zero-to-inf, r=BoxyUwU
change `NormalizesTo` to fully structurally normalize

notes in https://hackmd.io/wZ016dE4QKGIhrOnHLlThQ

need to also update the dev-guide once this PR lands. in short, the setup is now as follows:

`normalizes-to` internally implements one step normalization, applying that normalization to the `goal.predicate.term` causes the projected term to get recursively normalized. With this `normalizes-to` normalizes until the projected term is rigid, meaning that we normalize as many steps necessary, but at least 1.

To handle rigid aliases, we add another candidate only if the 1 to inf step normalization failed. With this `normalizes-to` is now full structural normalization. We can now change `AliasRelate` to simply emit `normalizes-to` goals for the rhs and lhs.

This avoids the concerns from https://github.com/rust-lang/trait-system-refactor-initiative/issues/103 and generally feels cleaner
2024-04-04 21:16:56 -04:00
Michael Goulet
55e46612c1 Force move async-closures that are FnOnce to make their inner coroutines also move 2024-04-04 19:44:51 -04:00
Michael Goulet
3d9d5d7c96 Actually use the inferred ClosureKind from signature inference in coroutine-closures 2024-04-04 19:44:35 -04:00
Matthew Maurer
b53a0f2c9e CFI: Add test for call_once addr taken
One of the proposed ways to reduce the non-passed argument erasure would
cause this test to fail. Adding this now ensures that any attempt to
reduce non-passed argument erasure won't make the same mistake.
2024-04-04 22:06:58 +00:00
bors
a4b11c8e60 Auto merge of #121394 - oli-obk:define_opaque_types, r=compiler-errors
some smaller DefiningOpaqueTypes::No -> Yes switches

r? `@compiler-errors`

These are some easy cases, so let's get them out of the way first.
I added tests exercising the specialization code paths that I believe weren't tested so far.

follow-up to https://github.com/rust-lang/rust/pull/117348
2024-04-04 17:42:07 +00:00
Oli Scherer
4e8d2f0040 Add regression test 2024-04-04 15:45:50 +00:00
Oli Scherer
0183d92df0 Allow defining opaque types when checking const equality bounds 2024-04-04 15:43:02 +00:00
bors
0fd571286e Auto merge of #123377 - oli-obk:private_projection, r=compiler-errors
Only inspect user-written predicates for privacy concerns

fixes #123288

Previously we looked at the elaborated predicates, which, due to adding various bounds on fields, end up requiring trivially true bounds. But these bounds can contain private types, which the privacy visitor then found and errored about.
2024-04-04 15:39:00 +00:00
Oli Scherer
29fba9f994 Add regression test 2024-04-04 15:15:21 +00:00
Oli Scherer
8e226e092e Add some regression tests for opaque types and const generics 2024-04-04 15:02:27 +00:00
Oli Scherer
ba316a902d amend to Switch can_eq and can_sub to DefineOpaqueTypes::Yes 2024-04-04 14:53:31 +00:00
Oli Scherer
83bd12c70f Only inspect user-written predicates for privacy concerns 2024-04-04 14:43:44 +00:00
Oli Scherer
169a045dca Switch upcast projections to allowing opaque types and add a test showing it works.
The old solver was already ICEing on this test before this change
2024-04-04 14:25:50 +00:00
Oli Scherer
cdcca7e8f4 Switch can_eq and can_sub to DefineOpaqueTypes::Yes
They are mostly used in diagnostics anyway
2024-04-04 14:25:45 +00:00
Matthias Krüger
ad300b6738
Rollup merge of #123431 - slanterns:literal_byte_character_c_string_stabilize, r=dtolnay
Stabilize `proc_macro_byte_character` and `proc_macro_c_str_literals`

This PR stabilizes `proc_macro_byte_character` and `proc_macro_c_str_literals`:

```rust
// proc_macro::Literal

impl Literal {
    pub fn byte_character(byte: u8) -> Literal;
    pub fn c_string(string: &CStr) -> Literal
}
```

<br>

Tracking issue: https://github.com/rust-lang/rust/issues/115268, https://github.com/rust-lang/rust/issues/119750.
Implementation PR: https://github.com/rust-lang/rust/pull/112711, https://github.com/rust-lang/rust/pull/119651.

FCPs already completed in their respective tracking issues.

Closes https://github.com/rust-lang/rust/issues/115268. Closes https://github.com/rust-lang/rust/issues/119750.

r? libs-api
2024-04-04 14:51:18 +02:00
Matthias Krüger
f254ab08f1
Rollup merge of #123397 - krtab:foreign_fn_qualif_diag, r=petrochenkov
Fix diagnostic for qualifier in extern block

Closes: https://github.com/rust-lang/rust/issues/123306
2024-04-04 14:51:17 +02:00
Matthias Krüger
504a78e2f2
Rollup merge of #123324 - Nadrieril:false-edges2, r=matthewjasper
match lowering: make false edges more precise

When lowering match expressions, we add false edges to hide details of the lowering from borrowck. Morally we pretend we're testing the patterns (and guards) one after the other in order. See the tests for examples. Problem is, the way we implement this today is too coarse for deref patterns.

In deref patterns, a pattern like `deref [1, x]` matches on a `Vec` by creating a temporary to store the output of the call to `deref()` and then uses that to continue matching. Here the pattern has a binding, which we set up after the pre-binding block. Problem is, currently the false edges tell borrowck that the pre-binding block can be reached from a previous arm as well, so the `deref()` temporary may not be initialized. This triggers an error when we try to use the binding `x`.

We could call `deref()` a second time, but this opens the door to soundness issues if the deref impl is weird. Instead in this PR I rework false edges a little bit.

What we need from false edges is a (fake) path from each candidate to the next, specifically from candidate C's pre-binding block to next candidate D's pre-binding block. Today, we link the pre-binding blocks directly. In this PR, I link them indirectly by choosing an earlier node on D's success path. Specifically, I choose the earliest block on D's success path that doesn't make a loop (if I chose e.g. the start block of the whole match (which is on the success path of all candidates), that would make a loop). This turns out to be rather straightforward to implement.

r? `@matthewjasper` if you have the bandwidth, otherwise let me know
2024-04-04 14:51:16 +02:00
Matthias Krüger
7c2d4eaf92
Rollup merge of #123218 - compiler-errors:synthetic-hir-parent, r=petrochenkov
Add test for getting parent HIR for synthetic HIR node

Fixes #122991, which was actually fixed by #123415
2024-04-04 14:51:16 +02:00
Matthias Krüger
0b54db7e3f
Rollup merge of #122448 - high-cloud:move-hir-tree, r=oli-obk
Port hir-tree run-make test to ui test

As part of #121876

cc `@jieyouxu`
2024-04-04 14:51:15 +02:00
Matthias Krüger
d5a657c95c
Rollup merge of #121546 - gurry:121473-ice-sizeof-mir-op, r=oli-obk
Error out of layout calculation if a non-last struct field is unsized

Fixes #121473
Fixes #123152
2024-04-04 14:51:14 +02:00
Yaodong Yang
2575b8e79c move hir-tree test from run-make to ui test 2024-04-04 18:43:26 +08:00
lcnr
92b280ce81 normalizes-to change from '1' to '0 to inf' steps 2024-04-04 12:39:58 +02:00
Gurinder Singh
313714331a Error out of layout calculation if a non-last struct field is unsized
Fixes an ICE that occurs when a struct with an unsized field
at a non-last position is const evaluated.
2024-04-04 15:50:36 +05:30
Oli Scherer
b8bd981545 Specialization already rejects defining opaque types 2024-04-04 10:01:45 +00:00
Arthur Carcano
109daa2d4b Fix diagnostic for qualifier in extern block
Closes: https://github.com/rust-lang/rust/issues/123306
2024-04-04 11:58:38 +02:00
Oli Scherer
769ab55558 Add regression test 2024-04-04 09:37:25 +00:00
bors
4c6c629866 Auto merge of #115538 - lcnr:fn-def-wf, r=compiler-errors
check `FnDef` return type for WF

better version of #106807, fixes #84533 (mostly). It's not perfect given that we still ignore WF requirements involving bound regions but I wasn't able to quickly write an example, so even if theoretically exploitable, it should be far harder to trigger.

This is strictly more restrictive than checking the return type for WF as part of the builtin `FnDef: FnOnce` impl (#106807) and moving to this approach in the future will not break any code.

~~It also agrees with my theoretical view of how this should behave~~

r? types
2024-04-04 08:43:53 +00:00
bors
29fe618f75 Auto merge of #123052 - maurer:addr-taken, r=compiler-errors
CFI: Support function pointers for trait methods

Adds support for both CFI and KCFI for function pointers to trait methods by attaching both concrete and abstract types to functions.

KCFI does this through generation of a `ReifyShim` on any function pointer for a method that could go into a vtable, and keeping this separate from `ReifyShim`s that are *intended* for vtable us by setting a `ReifyReason` on them.

CFI does this by setting both the concrete and abstract type on every instance.

This should land after #123024 or a similar PR, as it diverges the implementation of CFI vs KCFI.

r? `@compiler-errors`
2024-04-04 06:40:30 +00:00
lcnr
d99c775feb unconstrained NormalizesTo term for opaques 2024-04-04 07:47:22 +02:00
bors
43f4f2a3b1 Auto merge of #119820 - lcnr:leak-check-2, r=jackh726
instantiate higher ranked goals outside of candidate selection

This PR modifies `evaluate` to more eagerly instantiate higher-ranked goals, preventing the `leak_check` during candidate selection from detecting placeholder errors involving that binder.

For a general background regarding higher-ranked region solving and the leak check, see https://hackmd.io/qd9Wp03cQVy06yOLnro2Kg.

> The first is something called the **leak check**. You can think of it as a "quick and dirty" approximation for the region check, which will come later. The leak check detects some kinds of errors early, essentially deciding between "this set of outlives constraints are guaranteed to result in an error eventually" or "this set of outlives constraints may be solvable".

## The ideal future

We would like to end up with the following idealized design to handle universal binders:
```rust
fn enter_forall<'tcx, T, R>(
    forall: Binder<'tcx, T>,
    f: impl FnOnce(T) -> R,
) -> R {
    let new_universe = infcx.increment_universe_index();
    let value = instantiate_binder_with_placeholders_in(new_universe, forall);

    let result = f(value);

    eagerly_handle_higher_ranked_region_constraints_in(new_universe);
    infcx.decrement_universe_index();

    assert!(!result.has_placeholders_in_or_above(new_universe));
    result
}
```

That is, when universally instantiating a binder, anything using the placeholders has to happen inside of a limited scope (the closure `f`). After this closure has completed, all constraints involving placeholders are known.

We then handle any *external constraints* which name these placeholders. We destructure `TypeOutlives` constraints involving placeholders and eagerly handle any region constraints involving these placeholders. We do not return anything mentioning the placeholders created inside of this function to the caller.

Being able to eagerly handle *all* region constraints involving placeholders will be difficult due to complex `TypeOutlives` constraints, involving inference variables or alias types, and higher ranked implied bounds. The exact issues and possible solutions are out of scope of this FCP.

#### How does the leak check fit into this

The `leak_check` is an underapproximation of `eagerly_handle_higher_ranked_region_constraints_in`. It detects some kinds of errors involving placeholders from `new_universe`, but not all of them.

It only looks at region outlives constraints, ignoring `TypeOutlives`, and checks whether one of the following two conditions are met for **placeholders in or above `new_universe`**, in which case it results in an error:
- `'!p1: '!p2` a placeholder `'!p2` outlives a different placeholder `'!p1`
- `'!p1: '?2` an inference variable `'?2` outlives a placeholder `'!p1` *which it cannot name*

It does not handle all higher ranked region constraints, so we still return constraints involving placeholders from `new_universe` which are then (re)checked by `lexical_region_resolve` or MIR borrowck.

As we check higher ranked constraints in the full regionck anyways, the `leak_check` is not soundness critical. It's current only purpose is to move some higher ranked region errors earlier, enabling it to guide type inference and trait solving. Adding additional uses of the `leak_check` in the future would only strengthen inference and is therefore not breaking.

## Where do we use currently use the leak check

The `leak_check` is currently used in two places:

Coherence does not use a proper regionck, only relying on the `leak_check` called [at the end of the implicit negative overlap check](8b94152af6/compiler/rustc_trait_selection/src/traits/coherence.rs (L235-L238)). During coherence all parameters are instantiated with inference variables, so the only possible region errors are higher-ranked. We currently also sometimes make guesses when destructuring `TypeOutlives` constraints which can theoretically result in incorrect errors. This could result in overlapping impls.

We also use the `leak_check` [at the end of `fn evaluation_probe`](8b94152af6/compiler/rustc_trait_selection/src/traits/select/mod.rs (L607-L610)). This function is used during candidate assembly for `Trait` goals. Most notably we use [inside of `evaluate_candidate` during winnowing](0e4243538b/compiler/rustc_trait_selection/src/traits/select/mod.rs (L491-L502)). Conceptionally, it is as if we compute each candidate in a separate `enter_forall`.

## The current use in `fn evaluation_probe` is undesirable

Because we only instantiate a higher-ranked goal once inside of `fn evaluation_probe`, errors involving placeholders from that binder can impact selection. This results in inconsistent behavior ([playground](
*[playground](https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=dac60ebdd517201788899ffa77364831)*)):

```rust
trait Leak<'a> {}
impl Leak<'_>      for Box<u32> {}
impl Leak<'static> for Box<u16> {}

fn impls_leak<T: for<'a> Leak<'a>>() {}

trait IndirectLeak<'a> {}
impl<'a, T: Leak<'a>> IndirectLeak<'a> for T {}
fn impls_indirect_leak<T: for<'a> IndirectLeak<'a>>() {}

fn main() {
    // ok
    //
    // The `Box<u16>` impls fails the leak check,
    // meaning that we apply the `Box<u32>` impl.
    impls_leak::<Box<_>>();

    // error: type annotations needed
    //
    // While the `Box<u16>` impl would fail the leak check
    // we have already instantiated the binder while applying
    // the generic `IndirectLeak` impl, so during candidate
    // selection of `Leak` we do not detect the placeholder error.
    // Evaluation of `Box<_>: Leak<'!a>` is therefore ambiguous,
    // resulting in `for<'a> Box<_>: Leak<'a>` also being ambiguous.
    impls_indirect_leak::<Box<_>>();
}
```

We generally prefer `where`-bounds over implementations during candidate selection, both for [trait goals](11f32b73e0/compiler/rustc_trait_selection/src/traits/select/mod.rs (L1863-L1887)) and during [normalization](11f32b73e0/compiler/rustc_trait_selection/src/traits/project.rs (L184-L198)). However, we currently **do not** use the `leak_check` during candidate assembly in normalizing. This can result in inconsistent behavior:
```rust
trait Trait<'a> {
    type Assoc;
}
impl<'a, T> Trait<'a> for T {
    type Assoc = usize;
}

fn trait_bound<T: for<'a> Trait<'a>>() {}
fn projection_bound<T: for<'a> Trait<'a, Assoc = usize>>() {}

// A function with a trivial where-bound which is more
// restrictive than the impl.
fn function<T: Trait<'static, Assoc = usize>>() {
    // ok
    //
    // Proving `for<'a> T: Trait<'a>` using the where-bound results
    // in a leak check failure, so we use the more general impl,
    // causing this to succeed.
    trait_bound::<T>();

    // error
    //
    // Proving the `Projection` goal `for<'a> T: Trait<'a, Assoc = usize>`
    // does not use the leak check when trying the where-bound, causing us
    // to prefer it over the impl, resulting in a placeholder error.
    projection_bound::<T>();

    // error
    //
    // Trying to normalize the type `for<'a> fn(<T as Trait<'a>>::Assoc)`
    // only gets to `<T as Trait<'a>>::Assoc` once `'a` has been already
    // instantiated, causing us to prefer the where-bound over the impl
    // resulting in a placeholder error. Even if were were to also use the
    // leak check during candidate selection for normalization, this
    // case would still not compile.
    let _higher_ranked_norm: for<'a> fn(<T as Trait<'a>>::Assoc) = |_| ();
}
```

This is also likely to be more performant. It enables more caching in the new trait solver by simply [recursively calling the canonical query][new solver] after instantiating the higher-ranked goal.

It is also unclear how to add the leak check to normalization in the new solver. To handle https://github.com/rust-lang/trait-system-refactor-initiative/issues/1 `Projection` goals are implemented via `AliasRelate`. This again means that we instantiate the binder before ever normalizing any alias. Even if we were to avoid this, we lose the ability to [cache normalization by itself, ignoring the expected `term`](5bd5d214ef/compiler/rustc_trait_selection/src/solve/normalizes_to/mod.rs (L34-L49)). We cannot replace the `term` with an inference variable before instantiating the binder, as otherwise `for<'a> T: Trait<Assoc<'a> = &'a ()>` breaks. If we only replace the term after instantiating the binder, we cannot easily evaluate the goal in a separate context, as [we'd then lose the information necessary for the leak check](11f32b73e0/compiler/rustc_next_trait_solver/src/canonicalizer.rs (L230-L232)). Adding this information to the canonical input also seems non-trivial.

## Proposed solution

I propose to instantiate the binder outside of candidate assembly, causing placeholders from higher-ranked goals to get ignored while selecting their candidate. This mostly[^1] matches the [current behavior of the new solver][new solver]. The impact of this change is therefore as follows:

```rust
trait Leak<'a> {}
impl Leak<'_>      for Box<u32> {}
impl Leak<'static> for Box<u16> {}

fn impls_leak<T: for<'a> Leak<'a>>() {}

trait IndirectLeak<'a> {}
impl<'a, T: Leak<'a>> IndirectLeak<'a> for T {}
fn impls_indirect_leak<T: for<'a> IndirectLeak<'a>>() {}

fn guide_selection() {
    // ok -> ambiguous
    impls_leak::<Box<_>>();

    // ambiguous
    impls_indirect_leak::<Box<_>>();
}

trait Trait<'a> {
    type Assoc;
}
impl<'a, T> Trait<'a> for T {
    type Assoc = usize;
}

fn trait_bound<T: for<'a> Trait<'a>>() {}
fn projection_bound<T: for<'a> Trait<'a, Assoc = usize>>() {}

// A function which a trivial where-bound which is more
// restrictive than the impl.
fn function<T: Trait<'static, Assoc = usize>>() {
    // ok -> error
    trait_bound::<T>();

    // error
    projection_bound::<T>();

    // error
    let _higher_ranked_norm: for<'a> fn(<T as Trait<'a>>::Assoc) = |_| ();
}
```

This does not change the behavior if candidates have higher ranked nested goals, as in this case the `leak_check` causes the nested goal to result in an error ([playground](https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=a74c25300b23db9022226de99d8a2fa6)):
```rust
trait LeakCheckFailure<'a> {}
impl LeakCheckFailure<'static> for () {}

trait Trait<T> {}
impl Trait<u32> for () where for<'a> (): LeakCheckFailure<'a> {}
impl Trait<u16> for () {}
fn impls_trait<T: Trait<U>, U>() {}
fn main() {
    // ok
    //
    // It does not matter whether candidate assembly
    // considers the placeholders from higher-ranked goal.
    //
    // Either `for<'a> (): LeakCheckFailure<'a>` has no
    // applicable candidate or it has a single applicable candidate
    // when then later results in an error. This allows us to
    // infer `U` to `u16`.
    impls_trait::<(), _>()
}
```

## Impact on existing crates

This is a **breaking change**. [A crater run](https://github.com/rust-lang/rust/pull/119820#issuecomment-1926862174) found 17 regressed crates with 7 root causes.

For a full analysis of all affected crates, see https://gist.github.com/lcnr/7c1c652f30567048ea240554a36ed95c.

---

I believe this breakage to be acceptable and would merge this change. I am confident that the new position of the leak check matches our idealized future and cannot envision any other consistent alternative. Where possible, I intend to open PRs fixing/avoiding the regressions before landing this PR.

I originally intended to remove the `coherence_leak_check` lint in the same PR. However, while I am confident in the *position* of the leak check, deciding on its exact behavior is left as future work, cc #112999. This PR therefore only moves the leak check while keeping the lint when relying on it in coherence.

[new solver]: https://github.com/rust-lang/rust/blob/master/compiler/rustc_trait_selection/src/solve/eval_ctxt/mod.rs#L479-L484

[^1]: the new solver has a separate cause of inconsistent behavior rn https://github.com/rust-lang/trait-system-refactor-initiative/issues/53#issuecomment-1914310171

r? `@nikomatsakis`
2024-04-04 04:36:12 +00:00
Ben Kimock
b0b7c860e1 Teach MIR inliner query cycle avoidance about const_eval_select 2024-04-04 00:10:52 -04:00
bors
0accf4ec4c Auto merge of #123440 - jhpratt:rollup-yat6crk, r=jhpratt
Rollup of 4 pull requests

Successful merges:

 - #122356 (std::rand: fix dragonflybsd after #121942.)
 - #123093 (Add a nice header to our README.md)
 - #123307 (Fix f16 and f128 feature gating on different editions)
 - #123401 (Check `x86_64` size assertions on `aarch64`, too)

r? `@ghost`
`@rustbot` modify labels: rollup
2024-04-04 02:11:23 +00:00
Boxy
82789763c7 rebase 2024-04-04 02:14:57 +01:00
lcnr
2b67f0104a check FnDef return type for WF 2024-04-04 01:55:29 +01:00
Jacob Pratt
819568a7b4
Rollup merge of #123307 - tgross35:f16-f128-feature-gate-fix, r=petrochenkov
Fix f16 and f128 feature gating on different editions

Apply the fix from https://github.com/rust-lang/rust/issues/123282#issuecomment-2035063388 to correctly gates `f16` and `f128` in editions other than 2015
2024-04-03 20:17:05 -04:00
bors
b4acbe4233 Auto merge of #123240 - compiler-errors:assert-args-compat, r=fmease
Assert that args are actually compatible with their generics, rather than just their count

Right now we just check that the number of args is right, rather than actually checking the kinds. Uplift a helper fn that I wrote from trait selection to do just that. Found a couple bugs along the way.

r? `@lcnr` or `@fmease` (or anyone really lol)
2024-04-04 00:09:02 +00:00
Boxy
f090de8875 rebase oddity 2024-04-03 22:48:55 +01:00
Michael Goulet
f029602920 Tests for getting parent of synthetic HIR 2024-04-03 17:44:47 -04:00