Skip to content

Commit

Permalink
Auto merge of rust-lang#122412 - WaffleLapkin:if-we-ask-question-mark…
Browse files Browse the repository at this point in the history
…-operator-to-not-screw-with-inference-will-it-obey, r=<try>

Stop skewing inference in ?'s desugaring

**NB**: this is a breaking change (although arguably a bug fix) and as such shouldn't be taken lightly.

This changes `expr?`'s desugaring like so (simplified, see code for more info):
```rust
// old
match expr {
    Ok(val) => val,
    Err(err) => return Err(err),
}

// new
match expr {
    Ok(val) => val,
    Err(err) => core::convert::absurd(return Err(err)),
}

// core::convert
pub const fn absurd<T>(x: !) -> T { x }
```

This prevents `!` from the `return` from skewing inference:
```rust
// previously: ok (never type spontaneous decay skews inference, `T = ()`)
// with this pr: can't infer the type for `T`
Err(())?;
```

Fixes rust-lang#51125
Closes rust-lang#39216
  • Loading branch information
bors committed Apr 5, 2024
2 parents 385fa9d + b288d58 commit 645bb72
Show file tree
Hide file tree
Showing 26 changed files with 147 additions and 82 deletions.
30 changes: 22 additions & 8 deletions compiler/rustc_ast_lowering/src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1758,9 +1758,9 @@ impl<'hir> LoweringContext<'_, 'hir> {
/// ControlFlow::Break(residual) =>
/// #[allow(unreachable_code)]
/// // If there is an enclosing `try {...}`:
/// break 'catch_target Try::from_residual(residual),
/// absurd(break 'catch_target Try::from_residual(residual)),
/// // Otherwise:
/// return Try::from_residual(residual),
/// absurd(return Try::from_residual(residual)),
/// }
/// ```
fn lower_expr_try(&mut self, span: Span, sub_expr: &Expr) -> hir::ExprKind<'hir> {
Expand All @@ -1769,6 +1769,13 @@ impl<'hir> LoweringContext<'_, 'hir> {
span,
Some(self.allow_try_trait.clone()),
);

let absurd_allowed_span = self.mark_span_with_reason(
DesugaringKind::QuestionMark,
span,
Some(self.allow_convert_absurd.clone()),
);

let try_span = self.tcx.sess.source_map().end_point(span);
let try_span = self.mark_span_with_reason(
DesugaringKind::QuestionMark,
Expand Down Expand Up @@ -1810,7 +1817,7 @@ impl<'hir> LoweringContext<'_, 'hir> {

// `ControlFlow::Break(residual) =>
// #[allow(unreachable_code)]
// return Try::from_residual(residual),`
// absurd(return Try::from_residual(residual)),`
let break_arm = {
let residual_ident = Ident::with_dummy_span(sym::residual);
let (residual_local, residual_local_nid) = self.pat_ident(try_span, residual_ident);
Expand All @@ -1823,20 +1830,27 @@ impl<'hir> LoweringContext<'_, 'hir> {
);
let ret_expr = if let Some(catch_node) = self.catch_scope {
let target_id = Ok(self.lower_node_id(catch_node));
self.arena.alloc(self.expr(
self.expr(
try_span,
hir::ExprKind::Break(
hir::Destination { label: None, target_id },
Some(from_residual_expr),
),
))
)
} else {
self.arena.alloc(self.expr(try_span, hir::ExprKind::Ret(Some(from_residual_expr))))
self.expr(try_span, hir::ExprKind::Ret(Some(from_residual_expr)))
};
self.lower_attrs(ret_expr.hir_id, &attrs);

let absurd_expr = self.expr_call_lang_item_fn(
absurd_allowed_span,
hir::LangItem::Absurd,
arena_vec![self; ret_expr],
);

self.lower_attrs(absurd_expr.hir_id, &attrs);

let break_pat = self.pat_cf_break(try_span, residual_local);
self.arm(break_pat, ret_expr)
self.arm(break_pat, absurd_expr)
};

hir::ExprKind::Match(
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_ast_lowering/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ struct LoweringContext<'a, 'hir> {
node_id_to_local_id: NodeMap<hir::ItemLocalId>,

allow_try_trait: Lrc<[Symbol]>,
allow_convert_absurd: Lrc<[Symbol]>,
allow_gen_future: Lrc<[Symbol]>,
allow_async_iterator: Lrc<[Symbol]>,
allow_for_await: Lrc<[Symbol]>,
Expand Down Expand Up @@ -173,6 +174,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
impl_trait_defs: Vec::new(),
impl_trait_bounds: Vec::new(),
allow_try_trait: [sym::try_trait_v2, sym::yeet_desugar_details].into(),
allow_convert_absurd: [sym::convert_absurd].into(),
allow_gen_future: if tcx.features().async_fn_track_caller {
[sym::gen_future, sym::closure_track_caller].into()
} else {
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_hir/src/lang_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,8 @@ language_item_table! {
TryTraitBranch, sym::branch, branch_fn, Target::Method(MethodKind::Trait { body: false }), GenericRequirement::None;
TryTraitFromYeet, sym::from_yeet, from_yeet_fn, Target::Fn, GenericRequirement::None;

Absurd, sym::absurd, absurd, Target::Fn, GenericRequirement::Exact(1);

PointerLike, sym::pointer_like, pointer_like, Target::Trait, GenericRequirement::Exact(0);

ConstParamTy, sym::const_param_ty, const_param_ty_trait, Target::Trait, GenericRequirement::Exact(0);
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_span/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -354,6 +354,7 @@ symbols! {
abi_vectorcall,
abi_x86_interrupt,
abort,
absurd,
add,
add_assign,
add_with_overflow,
Expand Down Expand Up @@ -601,6 +602,7 @@ symbols! {
const_try,
constant,
constructor,
convert_absurd,
convert_identity,
copy,
copy_closures,
Expand Down
60 changes: 60 additions & 0 deletions library/core/src/convert/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,66 @@ pub const fn identity<T>(x: T) -> T {
x
}

/// Converts [`!`] (the never type) to any type.
///
/// This is possible because `!` is uninhabited (has no values), so this function can't actually
/// be ever called at runtime.
///
/// Even though `!` can be coerced to any type implicitly anyway (and indeed this function
/// implemented by just "returning" the argument), this is still useful, as this prevents the
/// fallback from happening during typechecking.
///
/// For example, this snippet type checks:
///
/// ```rust
/// let x: Result<_, ()> = Err(());
/// let y = match x {
/// Ok(v) => v,
/// Err(()) => return,
/// };
/// ```
///
/// This is a bit unexpected, because the type of `y` is seemingly unbound (indeed, it can be any
/// type). However, the `match` unifies type of `v` with type of `return` (which is `!`), so `y`
/// becomes `!` (or `()`, because of backwards compatibility shenanigans).
///
/// This can be avoided by adding `absurd`;
///
/// ```compile_fail,E0282
/// use core::convert::absurd;
///
/// let x: Result<_, ()> = Err(());
/// let y = match x { //~ error[E0282]: type annotations needed
/// Ok(v) => v,
///
/// // the call to `absurd` *is* unreachable, but it's still important for type check reasons
/// #[allow(unreachable_code)]
/// Err(()) => absurd(return),
/// };
/// ```
///
/// This might be handy when writing macros.
///
/// `absurd` can also be passed to higher order functions, just like any other function:
///
/// ```
/// #![feature(never_type)]
/// use core::convert::absurd;
///
/// let x: Result<_, !> = Ok(1);
/// let x: u32 = x.unwrap_or_else(absurd);
/// ```
///
/// [`!`]: ../../primitive.never.html
#[inline(always)]
#[lang = "absurd"]
#[unstable(feature = "convert_absurd", issue = "none")]
#[rustc_const_unstable(feature = "convert_absurd", issue = "none")]
#[cfg(not(bootstrap))]
pub const fn absurd<T>(x: !) -> T {
x
}

/// Used to do a cheap reference-to-reference conversion.
///
/// This trait is similar to [`AsMut`] which is used for converting between mutable references.
Expand Down
19 changes: 2 additions & 17 deletions src/bootstrap/src/core/build_steps/tool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -534,25 +534,10 @@ impl Step for Cargo {
)
}

fn make_run(run: RunConfig<'_>) {
run.builder.ensure(Cargo {
compiler: run.builder.compiler(run.builder.top_stage, run.builder.config.build),
target: run.target,
});
}
fn make_run(_run: RunConfig<'_>) {}

fn run(self, builder: &Builder<'_>) -> PathBuf {
let cargo_bin_path = builder.ensure(ToolBuild {
compiler: self.compiler,
target: self.target,
tool: "cargo",
mode: Mode::ToolRustc,
path: "src/tools/cargo",
source_type: SourceType::Submodule,
extra_features: Vec::new(),
allow_features: "",
});
cargo_bin_path
builder.initial_cargo.clone()
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/tools/opt-dist/src/training.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use humansize::BINARY;

const LLVM_PGO_CRATES: &[&str] = &[
"syn-1.0.89",
"cargo-0.60.0",
//"cargo-0.60.0",
"serde-1.0.136",
"ripgrep-13.0.0",
"regex-1.5.5",
Expand All @@ -19,7 +19,7 @@ const LLVM_PGO_CRATES: &[&str] = &[
const RUSTC_PGO_CRATES: &[&str] = &[
"externs",
"ctfe-stress-5",
"cargo-0.60.0",
//"cargo-0.60.0",
"token-stream-stress",
"match-stress",
"tuple-stress",
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/async-await/issue-67765-async-diagnostic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ async fn func<'a>() -> Result<(), &'a str> {

let b = &s[..];

Err(b)?; //~ ERROR cannot return value referencing local variable `s`
Err::<(), _>(b)?; //~ ERROR cannot return value referencing local variable `s`

Ok(())
}
4 changes: 2 additions & 2 deletions tests/ui/async-await/issue-67765-async-diagnostic.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ error[E0515]: cannot return value referencing local variable `s`
LL | let b = &s[..];
| - `s` is borrowed here
LL |
LL | Err(b)?;
| ^^^^^^^ returns a value referencing data owned by the current function
LL | Err::<(), _>(b)?;
| ^^^^^^^^^^^^^^^^ returns a value referencing data owned by the current function

error: aborting due to 1 previous error

Expand Down
4 changes: 2 additions & 2 deletions tests/ui/consts/try-operator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,15 @@

fn main() {
const fn result() -> Result<bool, ()> {
Err(())?;
Err::<(), _>(())?;
Ok(true)
}

const FOO: Result<bool, ()> = result();
assert_eq!(Err(()), FOO);

const fn option() -> Option<()> {
None?;
None::<()>?;
Some(())
}
const BAR: Option<()> = option();
Expand Down
16 changes: 8 additions & 8 deletions tests/ui/consts/try-operator.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ LL | #![feature(const_convert)]
error[E0015]: `?` cannot determine the branch of `Result<(), ()>` in constant functions
--> $DIR/try-operator.rs:10:9
|
LL | Err(())?;
| ^^^^^^^^
LL | Err::<(), _>(())?;
| ^^^^^^^^^^^^^^^^^
|
note: impl defined here, but it is not `const`
--> $SRC_DIR/core/src/result.rs:LL:COL
Expand All @@ -21,8 +21,8 @@ LL + #![feature(effects)]
error[E0015]: `?` cannot convert from residual of `Result<bool, ()>` in constant functions
--> $DIR/try-operator.rs:10:9
|
LL | Err(())?;
| ^^^^^^^^
LL | Err::<(), _>(())?;
| ^^^^^^^^^^^^^^^^^
|
note: impl defined here, but it is not `const`
--> $SRC_DIR/core/src/result.rs:LL:COL
Expand All @@ -35,8 +35,8 @@ LL + #![feature(effects)]
error[E0015]: `?` cannot determine the branch of `Option<()>` in constant functions
--> $DIR/try-operator.rs:18:9
|
LL | None?;
| ^^^^^
LL | None::<()>?;
| ^^^^^^^^^^^
|
note: impl defined here, but it is not `const`
--> $SRC_DIR/core/src/option.rs:LL:COL
Expand All @@ -49,8 +49,8 @@ LL + #![feature(effects)]
error[E0015]: `?` cannot convert from residual of `Option<()>` in constant functions
--> $DIR/try-operator.rs:18:9
|
LL | None?;
| ^^^^^
LL | None::<()>?;
| ^^^^^^^^^^^
|
note: impl defined here, but it is not `const`
--> $SRC_DIR/core/src/option.rs:LL:COL
Expand Down
2 changes: 0 additions & 2 deletions tests/ui/did_you_mean/compatible-variants.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,6 @@ LL + Some(())
error[E0308]: `?` operator has incompatible types
--> $DIR/compatible-variants.rs:35:5
|
LL | fn d() -> Option<()> {
| ---------- expected `Option<()>` because of return type
LL | c()?
| ^^^^ expected `Option<()>`, found `()`
|
Expand Down
10 changes: 5 additions & 5 deletions tests/ui/impl-trait/cross-return-site-inference.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,34 +2,34 @@

fn foo(b: bool) -> impl std::fmt::Debug {
if b {
return vec![42]
return vec![42];
}
[].into_iter().collect()
}

fn bar(b: bool) -> impl std::fmt::Debug {
if b {
return [].into_iter().collect()
return [].into_iter().collect();
}
vec![42]
}

fn bak(b: bool) -> impl std::fmt::Debug {
if b {
return std::iter::empty().collect()
return std::iter::empty().collect();
}
vec![42]
}

fn baa(b: bool) -> impl std::fmt::Debug {
if b {
return [42].into_iter().collect()
return [42].into_iter().collect();
}
vec![]
}

fn muh() -> Result<(), impl std::fmt::Debug> {
Err("whoops")?;
Err::<(), _>("whoops")?;
Ok(())
//~^ ERROR type annotations needed
}
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/inference/cannot-infer-closure.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
fn main() {
let x = |a: (), b: ()| {
Err(a)?;
Err::<(), _>(a)?;
Ok(b)
//~^ ERROR type annotations needed
};
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
error[E0308]: `?` operator has incompatible types
--> $DIR/issue-51632-try-desugar-incompatible-types.rs:8:5
|
LL | fn forbidden_narratives() -> Result<isize, ()> {
| ----------------- expected `Result<isize, ()>` because of return type
LL | missing_discourses()?
| ^^^^^^^^^^^^^^^^^^^^^ expected `Result<isize, ()>`, found `isize`
|
Expand Down
4 changes: 2 additions & 2 deletions tests/ui/label/label_break_value_desugared_break.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,14 @@
fn main() {
let _: Result<(), ()> = try {
'foo: {
Err(())?;
Err::<(), _>(())?;
break 'foo;
}
};

'foo: {
let _: Result<(), ()> = try {
Err(())?;
Err::<(), _>(())?;
break 'foo;
};
}
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/parser/try-with-nonterminal-block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ macro_rules! create_try {

fn main() {
let x: Option<&str> = create_try! {{
None?;
None::<()>?;
"Hello world"
}};

Expand Down
Loading

0 comments on commit 645bb72

Please sign in to comment.