Skip to content
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

Stop skewing inference in ?'s desugaring #122412

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"function is implemented"

/// 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
traviscross marked this conversation as resolved.
Show resolved Hide resolved
/// 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),
traviscross marked this conversation as resolved.
Show resolved Hide resolved
/// };
/// ```
///
/// 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
Loading