bjoernager/rust
bjoernager/rust
1
Fork 0
Code Activity

Auto merge of #89841 - cormacrelf:let-else-typed, r=nagisa

Browse source 
Implement let-else type annotations natively

Tracking issue: #87335

Fixes #89688, fixes #89807, edit: fixes  #89960 as well

As explained in https://github.com/rust-lang/rust/issues/89688#issuecomment-940405082, the previous desugaring moved the let-else scrutinee into a dummy variable, which meant if you wanted to refer to it again in the else block, it had moved.

This introduces a new hir type, ~~`hir::LetExpr`~~ `hir::Let`, which takes over all the fields of `hir::ExprKind::Let(...)` and adds an optional type annotation. The `hir::Let` is then treated like a `hir::Local` when type checking a function body, specifically:

* `GatherLocalsVisitor` overrides a new `Visitor::visit_let_expr` and does pretty much exactly what it does for `visit_local`, assigning a local type to the `hir::Let` ~~(they could be deduplicated but they are right next to each other, so at least we know they're the same)~~
* It reuses the code in `check_decl_local` to typecheck the `hir::Let`, simply returning 'bool' for the expression type after doing that.

* ~~`FnCtxt::check_expr_let` passes this local type in to `demand_scrutinee_type`, and then imitates check_decl_local's pattern checking~~
* ~~`demand_scrutinee_type` (the blindest change for me, please give this extra scrutiny) uses this local type instead of of creating a new one~~
    * ~~Just realised the `check_expr_with_needs` was passing NoExpectation further down, need to pass the type there too. And apparently this Expectation API already exists.~~

Some other misc notes:

* ~~Is the clippy code supposed to be autoformatted? I tried not to give huge diffs but maybe some rustfmt changes simply haven't hit it yet.~~
* in `rustc_ast_lowering/src/block.rs`, I noticed some existing `self.alias_attrs()` calls in `LoweringContext::lower_stmts` seem to be copying attributes from the lowered locals/etc to the statements. Is that right? I'm new at this, I don't know.
This commit is contained in:
bors 2021-12-17 22:12:34 +00:00
commit dde825db46
46 changed files with 900 additions and 142 deletions
Download patch file Download diff file Expand all files Collapse all files

13
compiler/rustc_typeck/src/check/expr.rs
View file

@ -300,7 +300,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
}
ExprKind::Ret(ref expr_opt) => self.check_expr_return(expr_opt.as_deref(), expr),
ExprKind::Let(pat, let_expr, _) => self.check_expr_let(let_expr, pat),
ExprKind::Let(let_expr) => self.check_expr_let(let_expr),
ExprKind::Loop(body, _, source, _) => {
self.check_expr_loop(body, source, expected, expr)
}
@ -1044,10 +1044,13 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
}
fn check_expr_let(&self, expr: &'tcx hir::Expr<'tcx>, pat: &'tcx hir::Pat<'tcx>) -> Ty<'tcx> {
self.warn_if_unreachable(expr.hir_id, expr.span, "block in `let` expression");
let expr_ty = self.demand_scrutinee_type(expr, pat.contains_explicit_ref_binding(), false);
self.check_pat_top(pat, expr_ty, Some(expr.span), true);
fn check_expr_let(&self, let_expr: &'tcx hir::Let<'tcx>) -> Ty<'tcx> {
// for let statements, this is done in check_stmt
let init = let_expr.init;
self.warn_if_unreachable(init.hir_id, init.span, "block in `let` expression");
// otherwise check exactly as a let statement
self.check_decl(let_expr.into());
// but return a bool, for this is a boolean expression
self.tcx.types.bool
}

45
compiler/rustc_typeck/src/check/fn_ctxt/checks.rs
View file

@ -1,5 +1,6 @@
use crate::astconv::AstConv;
use crate::check::coercion::CoerceMany;
use crate::check::gather_locals::Declaration;
use crate::check::method::MethodCallee;
use crate::check::Expectation::*;
use crate::check::TupleArgumentsFlag::*;
@ -538,16 +539,17 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
pub fn check_decl_initializer(
&self,
local: &'tcx hir::Local<'tcx>,
hir_id: hir::HirId,
pat: &'tcx hir::Pat<'tcx>,
init: &'tcx hir::Expr<'tcx>,
) -> Ty<'tcx> {
// FIXME(tschottdorf): `contains_explicit_ref_binding()` must be removed
// for #42640 (default match binding modes).
//
// See #44848.
let ref_bindings = local.pat.contains_explicit_ref_binding();
let ref_bindings = pat.contains_explicit_ref_binding();
let local_ty = self.local_ty(init.span, local.hir_id).revealed_ty;
let local_ty = self.local_ty(init.span, hir_id).revealed_ty;
if let Some(m) = ref_bindings {
// Somewhat subtle: if we have a `ref` binding in the pattern,
// we want to avoid introducing coercions for the RHS. This is
@ -565,29 +567,33 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
}
/// Type check a `let` statement.
pub fn check_decl_local(&self, local: &'tcx hir::Local<'tcx>) {
pub(in super::super) fn check_decl(&self, decl: Declaration<'tcx>) {
// Determine and write the type which we'll check the pattern against.
let ty = self.local_ty(local.span, local.hir_id).decl_ty;
self.write_ty(local.hir_id, ty);
let decl_ty = self.local_ty(decl.span, decl.hir_id).decl_ty;
self.write_ty(decl.hir_id, decl_ty);
// Type check the initializer.
if let Some(ref init) = local.init {
let init_ty = self.check_decl_initializer(local, &init);
self.overwrite_local_ty_if_err(local, ty, init_ty);
if let Some(ref init) = decl.init {
let init_ty = self.check_decl_initializer(decl.hir_id, decl.pat, &init);
self.overwrite_local_ty_if_err(decl.hir_id, decl.pat, decl_ty, init_ty);
}
// Does the expected pattern type originate from an expression and what is the span?
let (origin_expr, ty_span) = match (local.ty, local.init) {
let (origin_expr, ty_span) = match (decl.ty, decl.init) {
(Some(ty), _) => (false, Some(ty.span)), // Bias towards the explicit user type.
(_, Some(init)) => (true, Some(init.span)), // No explicit type; so use the scrutinee.
_ => (false, None), // We have `let $pat;`, so the expected type is unconstrained.
};
// Type check the pattern. Override if necessary to avoid knock-on errors.
self.check_pat_top(&local.pat, ty, ty_span, origin_expr);
let pat_ty = self.node_ty(local.pat.hir_id);
self.overwrite_local_ty_if_err(local, ty, pat_ty);
self.check_pat_top(&decl.pat, decl_ty, ty_span, origin_expr);
let pat_ty = self.node_ty(decl.pat.hir_id);
self.overwrite_local_ty_if_err(decl.hir_id, decl.pat, decl_ty, pat_ty);
}
/// Type check a `let` statement.
pub fn check_decl_local(&self, local: &'tcx hir::Local<'tcx>) {
self.check_decl(local.into());
}
pub fn check_stmt(&self, stmt: &'tcx hir::Stmt<'tcx>, is_last: bool) {
@ -891,17 +897,18 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
fn overwrite_local_ty_if_err(
&self,
local: &'tcx hir::Local<'tcx>,
hir_id: hir::HirId,
pat: &'tcx hir::Pat<'tcx>,
decl_ty: Ty<'tcx>,
ty: Ty<'tcx>,
) {
if ty.references_error() {
// Override the types everywhere with `err()` to avoid knock on errors.
self.write_ty(local.hir_id, ty);
self.write_ty(local.pat.hir_id, ty);
self.write_ty(hir_id, ty);
self.write_ty(pat.hir_id, ty);
let local_ty = LocalTy { decl_ty, revealed_ty: ty };
self.locals.borrow_mut().insert(local.hir_id, local_ty);
self.locals.borrow_mut().insert(local.pat.hir_id, local_ty);
self.locals.borrow_mut().insert(hir_id, local_ty);
self.locals.borrow_mut().insert(pat.hir_id, local_ty);
}
}

65
compiler/rustc_typeck/src/check/gather_locals.rs
View file

@ -7,6 +7,31 @@ use rustc_middle::ty::Ty;
use rustc_span::Span;
use rustc_trait_selection::traits;
/// A declaration is an abstraction of [hir::Local] and [hir::Let].
///
/// It must have a hir_id, as this is how we connect gather_locals to the check functions.
pub(super) struct Declaration<'a> {
pub hir_id: hir::HirId,
pub pat: &'a hir::Pat<'a>,
pub ty: Option<&'a hir::Ty<'a>>,
pub span: Span,
pub init: Option<&'a hir::Expr<'a>>,
}
impl<'a> From<&'a hir::Local<'a>> for Declaration<'a> {
fn from(local: &'a hir::Local<'a>) -> Self {
let hir::Local { hir_id, pat, ty, span, init, .. } = *local;
Declaration { hir_id, pat, ty, span, init }
}
}
impl<'a> From<&'a hir::Let<'a>> for Declaration<'a> {
fn from(let_expr: &'a hir::Let<'a>) -> Self {
let hir::Let { hir_id, pat, ty, span, init } = *let_expr;
Declaration { hir_id, pat, ty, span, init: Some(init) }
}
}
pub(super) struct GatherLocalsVisitor<'a, 'tcx> {
fcx: &'a FnCtxt<'a, 'tcx>,
// parameters are special cases of patterns, but we want to handle them as
@ -41,18 +66,12 @@ impl<'a, 'tcx> GatherLocalsVisitor<'a, 'tcx> {
}
}
}
}
impl<'a, 'tcx> Visitor<'tcx> for GatherLocalsVisitor<'a, 'tcx> {
type Map = intravisit::ErasedMap<'tcx>;
fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
NestedVisitorMap::None
}
// Add explicitly-declared locals.
fn visit_local(&mut self, local: &'tcx hir::Local<'tcx>) {
let local_ty = match local.ty {
/// Allocates a [LocalTy] for a declaration, which may have a type annotation. If it does have
/// a type annotation, then the LocalTy stored will be the resolved type. This may be found
/// again during type checking by querying [FnCtxt::local_ty] for the same hir_id.
fn declare(&mut self, decl: Declaration<'tcx>) {
let local_ty = match decl.ty {
Some(ref ty) => {
let o_ty = self.fcx.to_ty(&ty);
@ -68,16 +87,34 @@ impl<'a, 'tcx> Visitor<'tcx> for GatherLocalsVisitor<'a, 'tcx> {
}
None => None,
};
self.assign(local.span, local.hir_id, local_ty);
self.assign(decl.span, decl.hir_id, local_ty);
debug!(
"local variable {:?} is assigned type {}",
local.pat,
self.fcx.ty_to_string(&*self.fcx.locals.borrow().get(&local.hir_id).unwrap().decl_ty)
decl.pat,
self.fcx.ty_to_string(&*self.fcx.locals.borrow().get(&decl.hir_id).unwrap().decl_ty)
);
}
}
impl<'a, 'tcx> Visitor<'tcx> for GatherLocalsVisitor<'a, 'tcx> {
type Map = intravisit::ErasedMap<'tcx>;
fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
NestedVisitorMap::None
}
// Add explicitly-declared locals.
fn visit_local(&mut self, local: &'tcx hir::Local<'tcx>) {
self.declare(local.into());
intravisit::walk_local(self, local);
}
fn visit_let_expr(&mut self, let_expr: &'tcx hir::Let<'tcx>) {
self.declare(let_expr.into());
intravisit::walk_let_expr(self, let_expr);
}
fn visit_param(&mut self, param: &'tcx hir::Param<'tcx>) {
let old_outermost_fn_param_pat = self.outermost_fn_param_pat.replace(param.ty_span);
intravisit::walk_param(self, param);

4
compiler/rustc_typeck/src/expr_use_visitor.rs
View file

@ -229,8 +229,8 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> {
}
}
hir::ExprKind::Let(pat, ref expr, _) => {
self.walk_local(expr, pat, |t| t.borrow_expr(expr, ty::ImmBorrow));
hir::ExprKind::Let(hir::Let { pat, init, .. }) => {
self.walk_local(init, pat, |t| t.borrow_expr(init, ty::ImmBorrow));
}
hir::ExprKind::Match(ref discr, arms, _) => {