From f1f45f9191d60c52dbedec717aee0de4a0580bcc Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Fri, 21 Feb 2020 16:56:34 +0100 Subject: [PATCH] Fix handling of const patterns E.g. in `match x { None => ... }`, `None` is a path pattern (resolving to the option variant), not a binding. To determine this, we need to try to resolve the name during lowering. This isn't too hard since we already need to resolve names for macro expansion anyway (though maybe a bit hacky). Fixes #1618. --- crates/ra_hir_def/src/adt.rs | 1 + crates/ra_hir_def/src/body/lower.rs | 38 +++++++++++++++++++-- crates/ra_hir_def/src/expr.rs | 2 +- crates/ra_hir_ty/src/infer/pat.rs | 4 ++- crates/ra_hir_ty/src/tests/patterns.rs | 46 +++++++++++++++++++++++++- 5 files changed, 85 insertions(+), 6 deletions(-) diff --git a/crates/ra_hir_def/src/adt.rs b/crates/ra_hir_def/src/adt.rs index 985f409e873..2bdfc2b8d12 100644 --- a/crates/ra_hir_def/src/adt.rs +++ b/crates/ra_hir_def/src/adt.rs @@ -174,6 +174,7 @@ impl HasChildSource for VariantId { } } +#[derive(Debug, Copy, Clone, PartialEq, Eq)] pub enum StructKind { Tuple, Record, diff --git a/crates/ra_hir_def/src/body/lower.rs b/crates/ra_hir_def/src/body/lower.rs index b1626fa1160..b3fb6d452aa 100644 --- a/crates/ra_hir_def/src/body/lower.rs +++ b/crates/ra_hir_def/src/body/lower.rs @@ -15,6 +15,7 @@ use ra_syntax::{ use test_utils::tested_by; use crate::{ + adt::StructKind, body::{Body, BodySourceMap, Expander, PatPtr}, builtin_type::{BuiltinFloat, BuiltinInt}, db::DefDatabase, @@ -22,11 +23,12 @@ use crate::{ ArithOp, Array, BinaryOp, BindingAnnotation, CmpOp, Expr, ExprId, Literal, LogicOp, MatchArm, Ordering, Pat, PatId, RecordFieldPat, RecordLitField, Statement, }, + item_scope::BuiltinShadowMode, path::GenericArgs, path::Path, type_ref::{Mutability, TypeRef}, - ConstLoc, ContainerId, DefWithBodyId, EnumLoc, FunctionLoc, Intern, ModuleDefId, StaticLoc, - StructLoc, TraitLoc, TypeAliasLoc, UnionLoc, + AdtId, ConstLoc, ContainerId, DefWithBodyId, EnumLoc, FunctionLoc, Intern, ModuleDefId, + StaticLoc, StructLoc, TraitLoc, TypeAliasLoc, UnionLoc, }; pub(super) fn lower( @@ -571,7 +573,37 @@ where let name = bp.name().map(|nr| nr.as_name()).unwrap_or_else(Name::missing); let annotation = BindingAnnotation::new(bp.is_mutable(), bp.is_ref()); let subpat = bp.pat().map(|subpat| self.collect_pat(subpat)); - Pat::Bind { name, mode: annotation, subpat } + if annotation == BindingAnnotation::Unannotated && subpat.is_none() { + // This could also be a single-segment path pattern. To + // decide that, we need to try resolving the name. + let (resolved, _) = self.expander.crate_def_map.resolve_path( + self.db, + self.expander.module.local_id, + &name.clone().into(), + BuiltinShadowMode::Other, + ); + match resolved.take_values() { + Some(ModuleDefId::ConstId(_)) => Pat::Path(name.into()), + Some(ModuleDefId::EnumVariantId(_)) => { + // this is only really valid for unit variants, but + // shadowing other enum variants with a pattern is + // an error anyway + Pat::Path(name.into()) + } + Some(ModuleDefId::AdtId(AdtId::StructId(s))) + if self.db.struct_data(s).variant_data.kind() != StructKind::Record => + { + // Funnily enough, record structs *can* be shadowed + // by pattern bindings (but unit or tuple structs + // can't). + Pat::Path(name.into()) + } + // shadowing statics is an error as well, so we just ignore that case here + _ => Pat::Bind { name, mode: annotation, subpat }, + } + } else { + Pat::Bind { name, mode: annotation, subpat } + } } ast::Pat::TupleStructPat(p) => { let path = p.path().and_then(|path| self.expander.parse_path(path)); diff --git a/crates/ra_hir_def/src/expr.rs b/crates/ra_hir_def/src/expr.rs index 9707c55276a..66d0047173c 100644 --- a/crates/ra_hir_def/src/expr.rs +++ b/crates/ra_hir_def/src/expr.rs @@ -48,7 +48,7 @@ pub enum Literal { #[derive(Debug, Clone, Eq, PartialEq)] pub enum Expr { - /// This is produced if syntax tree does not have a required expression piece. + /// This is produced if the syntax tree does not have a required expression piece. Missing, Path(Path), If { diff --git a/crates/ra_hir_ty/src/infer/pat.rs b/crates/ra_hir_ty/src/infer/pat.rs index a495ecbfeb0..bf8ea192b7a 100644 --- a/crates/ra_hir_ty/src/infer/pat.rs +++ b/crates/ra_hir_ty/src/infer/pat.rs @@ -189,7 +189,9 @@ impl<'a, D: HirDatabase> InferenceContext<'a, D> { }; // use a new type variable if we got Ty::Unknown here let ty = self.insert_type_vars_shallow(ty); - self.unify(&ty, expected); + if !self.unify(&ty, expected) { + // FIXME record mismatch, we need to change the type of self.type_mismatches for that + } let ty = self.resolve_ty_as_possible(ty); self.write_pat_ty(pat, ty.clone()); ty diff --git a/crates/ra_hir_ty/src/tests/patterns.rs b/crates/ra_hir_ty/src/tests/patterns.rs index e25d6dbc43d..81d00c2afb8 100644 --- a/crates/ra_hir_ty/src/tests/patterns.rs +++ b/crates/ra_hir_ty/src/tests/patterns.rs @@ -1,4 +1,4 @@ -use super::infer; +use super::{infer, infer_with_mismatches}; use insta::assert_snapshot; use test_utils::covers; @@ -236,3 +236,47 @@ fn test(a1: A, o: Option) { "### ); } + +#[test] +fn infer_const_pattern() { + assert_snapshot!( + infer_with_mismatches(r#" +enum Option { None } +use Option::None; +struct Foo; +const Bar: usize = 1; + +fn test() { + let a: Option = None; + let b: Option = match a { + None => None, + }; + let _: () = match () { Foo => Foo }; // Expected mismatch + let _: () = match () { Bar => Bar }; // Expected mismatch +} +"#, true), + @r###" + [74; 75) '1': usize + [88; 310) '{ ...atch }': () + [98; 99) 'a': Option + [115; 119) 'None': Option + [129; 130) 'b': Option + [146; 183) 'match ... }': Option + [152; 153) 'a': Option + [164; 168) 'None': Option + [172; 176) 'None': Option + [193; 194) '_': () + [201; 224) 'match ... Foo }': Foo + [207; 209) '()': () + [212; 215) 'Foo': Foo + [219; 222) 'Foo': Foo + [255; 256) '_': () + [263; 286) 'match ... Bar }': usize + [269; 271) '()': () + [274; 277) 'Bar': usize + [281; 284) 'Bar': usize + [201; 224): expected (), got Foo + [263; 286): expected (), got usize + "### + ); +}