From b9b42e867066288f2cd1fc835c77a31e27439496 Mon Sep 17 00:00:00 2001 From: Artur Sinila Date: Thu, 14 Jul 2022 00:32:25 +0300 Subject: [PATCH 1/4] tests: add hover tests for const generics --- crates/ide/src/hover/tests.rs | 100 ++++++++++++++++++++++++++++++++++ 1 file changed, 100 insertions(+) diff --git a/crates/ide/src/hover/tests.rs b/crates/ide/src/hover/tests.rs index a1484fa19f..867d1f54d4 100644 --- a/crates/ide/src/hover/tests.rs +++ b/crates/ide/src/hover/tests.rs @@ -2958,6 +2958,106 @@ struct S$0T(T); ); } +#[test] +fn const_generic_positive_i8_literal() { + check( + r#" +struct Const; + +fn main() { + let v$0alue = Const::<1>; +} +"#, + expect![[r#" + *value* + + ```rust + let value: Const<1> + ``` + "#]], + ); +} + +#[test] +fn const_generic_zero_i8_literal() { + check( + r#" +struct Const; + +fn main() { + let v$0alue = Const::<0>; +} +"#, + expect![[r#" + *value* + + ```rust + let value: Const<0> + ``` + "#]], + ); +} + +#[test] +fn const_generic_negative_i8_literal() { + check( + r#" +struct Const; + +fn main() { + let v$0alue = Const::<-1>; +} +"#, + expect![[r#" + *value* + + ```rust + let value: Const<-1> + ``` + "#]], + ); +} + +#[test] +fn const_generic_bool_literal() { + check( + r#" +struct Const; + +fn main() { + let v$0alue = Const::; +} +"#, + expect![[r#" + *value* + + ```rust + let value: Const + ``` + "#]], + ); +} + +#[test] +fn const_generic_char_literal() { + check( + r#" +struct Const; + +fn main() { + let v$0alue = Const::<'🦀'>; +} +"#, + expect![[r#" + *value* + + ```rust + let value: Const<'🦀'> + ``` + "#]], + ); +} + #[test] fn hover_self_param_shows_type() { check( From a96f0aa7cdec0d5c1450fe7661dfda235bd5b94c Mon Sep 17 00:00:00 2001 From: Artur Sinila Date: Sun, 17 Jul 2022 04:18:53 +0300 Subject: [PATCH 2/4] feat: support negative const generic parameters * feat: support `bool` & `char` const generics --- crates/hir-def/src/body.rs | 2 +- crates/hir-def/src/body/lower.rs | 45 ++++---- crates/hir-def/src/expr.rs | 5 +- crates/hir-def/src/path/lower.rs | 10 +- crates/hir-def/src/type_ref.rs | 103 +++++++++++------- crates/hir-ty/src/consteval.rs | 33 +++--- crates/hir-ty/src/infer.rs | 2 +- crates/hir-ty/src/infer/expr.rs | 4 +- crates/hir-ty/src/infer/pat.rs | 14 ++- crates/hir-ty/src/interner.rs | 7 +- crates/hir-ty/src/lower.rs | 6 +- crates/hir-ty/src/tests/simple.rs | 4 +- .../src/handlers/type_mismatch.rs | 2 +- crates/ide/src/hover.rs | 30 ++--- lib/la-arena/src/lib.rs | 12 ++ 15 files changed, 153 insertions(+), 126 deletions(-) diff --git a/crates/hir-def/src/body.rs b/crates/hir-def/src/body.rs index 94210ab33f..5c4e7bc338 100644 --- a/crates/hir-def/src/body.rs +++ b/crates/hir-def/src/body.rs @@ -238,7 +238,7 @@ pub struct Mark { } /// The body of an item (function, const etc.). -#[derive(Debug, Eq, PartialEq)] +#[derive(Debug, Default, Eq, PartialEq)] pub struct Body { pub exprs: Arena, pub pats: Arena, diff --git a/crates/hir-def/src/body/lower.rs b/crates/hir-def/src/body/lower.rs index 049afa8227..37a0940a3f 100644 --- a/crates/hir-def/src/body/lower.rs +++ b/crates/hir-def/src/body/lower.rs @@ -1,7 +1,7 @@ //! Transforms `ast::Expr` into an equivalent `hir_def::expr::Expr` //! representation. -use std::{mem, sync::Arc}; +use std::{collections::HashMap, mem, sync::Arc}; use either::Either; use hir_expand::{ @@ -10,8 +10,6 @@ use hir_expand::{ name::{name, AsName, Name}, ExpandError, HirFileId, InFile, }; -use la_arena::Arena; -use profile::Count; use rustc_hash::FxHashMap; use syntax::{ ast::{ @@ -28,8 +26,8 @@ use crate::{ builtin_type::{BuiltinFloat, BuiltinInt, BuiltinUint}, db::DefDatabase, expr::{ - dummy_expr_id, Array, BindingAnnotation, Expr, ExprId, FloatTypeWrapper, Label, LabelId, - Literal, MatchArm, Pat, PatId, RecordFieldPat, RecordLitField, Statement, + Array, BindingAnnotation, Expr, ExprId, FloatTypeWrapper, Label, LabelId, Literal, + MatchArm, Pat, PatId, RecordFieldPat, RecordLitField, Statement, }, intern::Interned, item_scope::BuiltinShadowMode, @@ -82,24 +80,8 @@ pub(super) fn lower( params: Option, body: Option, ) -> (Body, BodySourceMap) { - ExprCollector { - db, - source_map: BodySourceMap::default(), - body: Body { - exprs: Arena::default(), - pats: Arena::default(), - labels: Arena::default(), - params: Vec::new(), - body_expr: dummy_expr_id(), - block_scopes: Vec::new(), - _c: Count::new(), - or_pats: Default::default(), - }, - expander, - name_to_pat_grouping: Default::default(), - is_lowering_inside_or_pat: false, - } - .collect(params, body) + let collector = ExprCollector::new(db, expander); + collector.collect(params, body) } struct ExprCollector<'a> { @@ -112,7 +94,18 @@ struct ExprCollector<'a> { is_lowering_inside_or_pat: bool, } -impl ExprCollector<'_> { +impl<'a> ExprCollector<'a> { + pub(crate) fn new(db: &'a dyn DefDatabase, expander: Expander) -> Self { + Self { + db, + expander, + body: Body::default(), + source_map: BodySourceMap::default(), + name_to_pat_grouping: HashMap::default(), + is_lowering_inside_or_pat: false, + } + } + fn collect( mut self, param_list: Option, @@ -197,7 +190,8 @@ impl ExprCollector<'_> { } fn collect_expr(&mut self, expr: ast::Expr) -> ExprId { - self.maybe_collect_expr(expr).unwrap_or_else(|| self.missing_expr()) + let expr_id = self.maybe_collect_expr(expr).unwrap_or_else(|| self.missing_expr()); + expr_id } /// Returns `None` if and only if the expression is `#[cfg]`d out. @@ -689,7 +683,6 @@ impl ExprCollector<'_> { }; let prev_def_map = mem::replace(&mut self.expander.def_map, def_map); let prev_local_module = mem::replace(&mut self.expander.module, module); - let mut statements: Vec<_> = block.statements().filter_map(|s| self.collect_stmt(s)).collect(); let tail = block.tail_expr().and_then(|e| self.maybe_collect_expr(e)); diff --git a/crates/hir-def/src/expr.rs b/crates/hir-def/src/expr.rs index 4295bacc46..c25aa0bcaf 100644 --- a/crates/hir-def/src/expr.rs +++ b/crates/hir-def/src/expr.rs @@ -13,7 +13,7 @@ //! See also a neighboring `body` module. use hir_expand::name::Name; -use la_arena::{Idx, RawIdx}; +use la_arena::Idx; use crate::{ builtin_type::{BuiltinFloat, BuiltinInt, BuiltinUint}, @@ -26,9 +26,6 @@ use crate::{ pub use syntax::ast::{ArithOp, BinaryOp, CmpOp, LogicOp, Ordering, RangeOp, UnaryOp}; pub type ExprId = Idx; -pub(crate) fn dummy_expr_id() -> ExprId { - ExprId::from_raw(RawIdx::from(!0)) -} pub type PatId = Idx; diff --git a/crates/hir-def/src/path/lower.rs b/crates/hir-def/src/path/lower.rs index b6a24cd4ab..6a15b1c085 100644 --- a/crates/hir-def/src/path/lower.rs +++ b/crates/hir-def/src/path/lower.rs @@ -1,6 +1,9 @@ //! Transforms syntax into `Path` objects, ideally with accounting for hygiene -use crate::{intern::Interned, type_ref::ConstScalarOrPath}; +use crate::{ + intern::Interned, + type_ref::{ConstScalar, ConstScalarOrPath}, +}; use either::Either; use hir_expand::name::{name, AsName}; @@ -181,7 +184,10 @@ pub(super) fn lower_generic_args( } } ast::GenericArg::ConstArg(arg) => { - let arg = ConstScalarOrPath::from_expr_opt(arg.expr()); + let arg = arg.expr().map_or( + ConstScalarOrPath::Scalar(ConstScalar::Unknown), + ConstScalarOrPath::from_expr, + ); args.push(GenericArg::Const(arg)) } } diff --git a/crates/hir-def/src/type_ref.rs b/crates/hir-def/src/type_ref.rs index 81a2f024b3..1916a6b3c5 100644 --- a/crates/hir-def/src/type_ref.rs +++ b/crates/hir-def/src/type_ref.rs @@ -5,10 +5,15 @@ use hir_expand::{ name::{AsName, Name}, AstId, InFile, }; -use std::{convert::TryInto, fmt::Write}; use syntax::ast::{self, HasName}; -use crate::{body::LowerCtx, intern::Interned, path::Path}; +use crate::{ + body::LowerCtx, + builtin_type::{BuiltinInt, BuiltinType, BuiltinUint}, + expr::Literal, + intern::Interned, + path::Path, +}; #[derive(Copy, Clone, PartialEq, Eq, Hash, Debug)] pub enum Mutability { @@ -177,7 +182,10 @@ impl TypeRef { // `hir_def::body::lower` to lower this into an `Expr` and then evaluate it at the // `hir_ty` level, which would allow knowing the type of: // let v: [u8; 2 + 2] = [0u8; 4]; - let len = ConstScalarOrPath::from_expr_opt(inner.expr()); + let len = inner.expr().map_or( + ConstScalarOrPath::Scalar(ConstScalar::Unknown), + ConstScalarOrPath::from_expr, + ); TypeRef::Array(Box::new(TypeRef::from_ast_opt(ctx, inner.ty())), len) } @@ -386,16 +394,9 @@ impl std::fmt::Display for ConstScalarOrPath { } impl ConstScalarOrPath { - pub(crate) fn from_expr_opt(expr: Option) -> Self { - match expr { - Some(x) => Self::from_expr(x), - None => Self::Scalar(ConstScalar::Unknown), - } - } - // FIXME: as per the comments on `TypeRef::Array`, this evaluation should not happen at this // parse stage. - fn from_expr(expr: ast::Expr) -> Self { + pub(crate) fn from_expr(expr: ast::Expr) -> Self { match expr { ast::Expr::PathExpr(p) => { match p.path().and_then(|x| x.segment()).and_then(|x| x.name_ref()) { @@ -403,22 +404,31 @@ impl ConstScalarOrPath { None => Self::Scalar(ConstScalar::Unknown), } } - ast::Expr::Literal(lit) => { - let lkind = lit.kind(); - match lkind { - ast::LiteralKind::IntNumber(num) - if num.suffix() == None || num.suffix() == Some("usize") => - { - Self::Scalar( - num.value() - .and_then(|v| v.try_into().ok()) - .map(ConstScalar::Usize) - .unwrap_or(ConstScalar::Unknown), - ) + ast::Expr::PrefixExpr(prefix_expr) => match prefix_expr.op_kind() { + Some(ast::UnaryOp::Neg) => { + let unsigned = prefix_expr + .expr() + .map_or(Self::Scalar(ConstScalar::Unknown), Self::from_expr); + // Add sign + match unsigned { + Self::Scalar(ConstScalar::UInt(num)) => { + Self::Scalar(ConstScalar::Int(-(num as i128))) + } + other => other, } - _ => Self::Scalar(ConstScalar::Unknown), } - } + _ => prefix_expr.expr().map_or(Self::Scalar(ConstScalar::Unknown), Self::from_expr), + }, + ast::Expr::Literal(literal) => Self::Scalar(match literal.kind() { + ast::LiteralKind::IntNumber(num) => { + num.value().map(ConstScalar::UInt).unwrap_or(ConstScalar::Unknown) + } + ast::LiteralKind::Char(c) => { + c.value().map(ConstScalar::Char).unwrap_or(ConstScalar::Unknown) + } + ast::LiteralKind::Bool(f) => ConstScalar::Bool(f), + _ => ConstScalar::Unknown, + }), _ => Self::Scalar(ConstScalar::Unknown), } } @@ -427,9 +437,10 @@ impl ConstScalarOrPath { /// A concrete constant value #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] pub enum ConstScalar { - // for now, we only support the trivial case of constant evaluating the length of an array - // Note that this is u64 because the target usize may be bigger than our usize - Usize(u64), + Int(i128), + UInt(u128), + Bool(bool), + Char(char), /// Case of an unknown value that rustc might know but we don't // FIXME: this is a hack to get around chalk not being able to represent unevaluatable @@ -439,21 +450,37 @@ pub enum ConstScalar { Unknown, } -impl std::fmt::Display for ConstScalar { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> Result<(), std::fmt::Error> { +impl ConstScalar { + pub fn builtin_type(&self) -> BuiltinType { match self { - ConstScalar::Usize(us) => us.fmt(f), - ConstScalar::Unknown => f.write_char('_'), + ConstScalar::UInt(_) | ConstScalar::Unknown => BuiltinType::Uint(BuiltinUint::U128), + ConstScalar::Int(_) => BuiltinType::Int(BuiltinInt::I128), + ConstScalar::Char(_) => BuiltinType::Char, + ConstScalar::Bool(_) => BuiltinType::Bool, } } } -impl ConstScalar { - /// Gets a target usize out of the ConstScalar - pub fn as_usize(&self) -> Option { - match self { - &ConstScalar::Usize(us) => Some(us), - _ => None, +impl From for ConstScalar { + fn from(literal: Literal) -> Self { + match literal { + Literal::Char(c) => Self::Char(c), + Literal::Bool(flag) => Self::Bool(flag), + Literal::Int(num, _) => Self::Int(num), + Literal::Uint(num, _) => Self::UInt(num), + _ => Self::Unknown, + } + } +} + +impl std::fmt::Display for ConstScalar { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> Result<(), std::fmt::Error> { + match self { + ConstScalar::Int(num) => num.fmt(f), + ConstScalar::UInt(num) => num.fmt(f), + ConstScalar::Bool(flag) => flag.fmt(f), + ConstScalar::Char(c) => write!(f, "'{c}'"), + ConstScalar::Unknown => f.write_str("{unknown}"), } } } diff --git a/crates/hir-ty/src/consteval.rs b/crates/hir-ty/src/consteval.rs index 4b58262dac..61183be04f 100644 --- a/crates/hir-ty/src/consteval.rs +++ b/crates/hir-ty/src/consteval.rs @@ -347,17 +347,6 @@ pub fn eval_const( } } -pub fn eval_usize(expr: Idx, mut ctx: ConstEvalCtx<'_>) -> Option { - if let Ok(ce) = eval_const(expr, &mut ctx) { - match ce { - ComputedExpr::Literal(Literal::Int(x, _)) => return x.try_into().ok(), - ComputedExpr::Literal(Literal::Uint(x, _)) => return x.try_into().ok(), - _ => {} - } - } - None -} - pub(crate) fn path_to_const( db: &dyn HirDatabase, resolver: &Resolver, @@ -406,19 +395,24 @@ pub fn unknown_const_as_generic(ty: Ty) -> GenericArg { } /// Interns a constant scalar with the given type -pub fn intern_scalar_const(value: ConstScalar, ty: Ty) -> Const { +pub fn intern_const_scalar_with_type(value: ConstScalar, ty: Ty) -> Const { ConstData { ty, value: ConstValue::Concrete(chalk_ir::ConcreteConst { interned: value }) } .intern(Interner) } /// Interns a possibly-unknown target usize -pub fn usize_const(value: Option) -> Const { - intern_scalar_const( - value.map(ConstScalar::Usize).unwrap_or(ConstScalar::Unknown), +pub fn usize_const(value: Option) -> Const { + intern_const_scalar_with_type( + value.map(ConstScalar::UInt).unwrap_or(ConstScalar::Unknown), TyBuilder::usize(), ) } +/// Interns a constant scalar with the default type +pub fn intern_const_scalar(value: ConstScalar) -> Const { + intern_const_scalar_with_type(value, TyBuilder::builtin(value.builtin_type())) +} + pub(crate) fn const_eval_recover( _: &dyn HirDatabase, _: &[String], @@ -463,7 +457,7 @@ pub(crate) fn eval_to_const<'a>( } } let body = ctx.body.clone(); - let ctx = ConstEvalCtx { + let mut ctx = ConstEvalCtx { db: ctx.db, owner: ctx.owner, exprs: &body.exprs, @@ -471,7 +465,12 @@ pub(crate) fn eval_to_const<'a>( local_data: HashMap::default(), infer: &ctx.result, }; - usize_const(eval_usize(expr, ctx)) + let computed_expr = eval_const(expr, &mut ctx); + let const_scalar = match computed_expr { + Ok(ComputedExpr::Literal(literal)) => literal.into(), + _ => ConstScalar::Unknown, + }; + intern_const_scalar_with_type(const_scalar, TyBuilder::usize()) } #[cfg(test)] diff --git a/crates/hir-ty/src/infer.rs b/crates/hir-ty/src/infer.rs index ecadfb6ce4..c7c8deaaee 100644 --- a/crates/hir-ty/src/infer.rs +++ b/crates/hir-ty/src/infer.rs @@ -605,10 +605,10 @@ impl<'a> InferenceContext<'a> { let data = c.data(Interner); match data.value { ConstValue::Concrete(cc) => match cc.interned { - hir_def::type_ref::ConstScalar::Usize(_) => c, hir_def::type_ref::ConstScalar::Unknown => { self.table.new_const_var(data.ty.clone()) } + _ => c, }, _ => c, } diff --git a/crates/hir-ty/src/infer/expr.rs b/crates/hir-ty/src/infer/expr.rs index 6dee0322a9..2f33467072 100644 --- a/crates/hir-ty/src/infer/expr.rs +++ b/crates/hir-ty/src/infer/expr.rs @@ -729,7 +729,7 @@ impl<'a> InferenceContext<'a> { let cur_elem_ty = self.infer_expr_inner(expr, &expected); coerce.coerce(self, Some(expr), &cur_elem_ty); } - consteval::usize_const(Some(items.len() as u64)) + consteval::usize_const(Some(items.len() as u128)) } &Array::Repeat { initializer, repeat } => { self.infer_expr_coerce(initializer, &Expectation::has_type(elem_ty)); @@ -766,7 +766,7 @@ impl<'a> InferenceContext<'a> { Literal::ByteString(bs) => { let byte_type = TyKind::Scalar(Scalar::Uint(UintTy::U8)).intern(Interner); - let len = consteval::usize_const(Some(bs.len() as u64)); + let len = consteval::usize_const(Some(bs.len() as u128)); let array_type = TyKind::Array(byte_type, len).intern(Interner); TyKind::Ref(Mutability::Not, static_lifetime(), array_type).intern(Interner) diff --git a/crates/hir-ty/src/infer/pat.rs b/crates/hir-ty/src/infer/pat.rs index dc86f696d4..327e463eec 100644 --- a/crates/hir-ty/src/infer/pat.rs +++ b/crates/hir-ty/src/infer/pat.rs @@ -11,6 +11,7 @@ use hir_def::{ use hir_expand::name::Name; use crate::{ + consteval::intern_const_scalar, infer::{BindingMode, Expectation, InferenceContext, TypeMismatch}, lower::lower_to_chalk_mutability, static_lifetime, ConcreteConst, ConstValue, Interner, Substitution, Ty, TyBuilder, TyExt, @@ -262,13 +263,18 @@ impl<'a> InferenceContext<'a> { if let &Some(slice_pat_id) = slice { let rest_pat_ty = match expected.kind(Interner) { TyKind::Array(_, length) => { - let length = match length.data(Interner).value { + let len = match length.data(Interner).value { ConstValue::Concrete(ConcreteConst { - interned: ConstScalar::Usize(length), - }) => length.checked_sub((prefix.len() + suffix.len()) as u64), + interned: ConstScalar::UInt(len), + }) => len.checked_sub((prefix.len() + suffix.len()) as u128), _ => None, }; - TyKind::Array(elem_ty.clone(), crate::consteval::usize_const(length)) + TyKind::Array( + elem_ty.clone(), + intern_const_scalar( + len.map_or(ConstScalar::Unknown, |len| ConstScalar::UInt(len)), + ), + ) } _ => TyKind::Slice(elem_ty.clone()), } diff --git a/crates/hir-ty/src/interner.rs b/crates/hir-ty/src/interner.rs index 07197d8a19..479521e8ba 100644 --- a/crates/hir-ty/src/interner.rs +++ b/crates/hir-ty/src/interner.rs @@ -257,12 +257,7 @@ impl chalk_ir::interner::Interner for Interner { c1: &Self::InternedConcreteConst, c2: &Self::InternedConcreteConst, ) -> bool { - match (c1, c2) { - (&ConstScalar::Usize(a), &ConstScalar::Usize(b)) => a == b, - // we were previously assuming this to be true, I'm not whether true or false on - // unknown values is safer. - (_, _) => true, - } + c1 == c2 } fn intern_generic_arg( diff --git a/crates/hir-ty/src/lower.rs b/crates/hir-ty/src/lower.rs index f9747f3b34..f62972b1eb 100644 --- a/crates/hir-ty/src/lower.rs +++ b/crates/hir-ty/src/lower.rs @@ -44,7 +44,9 @@ use syntax::{ast, SmolStr}; use crate::{ all_super_traits, - consteval::{intern_scalar_const, path_to_const, unknown_const, unknown_const_as_generic}, + consteval::{ + intern_const_scalar_with_type, path_to_const, unknown_const, unknown_const_as_generic, + }, db::HirDatabase, make_binders, mapping::ToChalk, @@ -1742,7 +1744,7 @@ pub(crate) fn const_or_path_to_chalk( debruijn: DebruijnIndex, ) -> Const { match value { - ConstScalarOrPath::Scalar(s) => intern_scalar_const(s.clone(), expected_ty), + ConstScalarOrPath::Scalar(s) => intern_const_scalar_with_type(s.clone(), expected_ty), ConstScalarOrPath::Path(n) => { let path = ModPath::from_segments(PathKind::Plain, Some(n.clone())); path_to_const(db, resolver, &path, mode, args, debruijn) diff --git a/crates/hir-ty/src/tests/simple.rs b/crates/hir-ty/src/tests/simple.rs index 5b08f55210..4d4795e933 100644 --- a/crates/hir-ty/src/tests/simple.rs +++ b/crates/hir-ty/src/tests/simple.rs @@ -3011,14 +3011,14 @@ struct TS(usize); fn main() { let x; [x,] = &[1,]; - //^^^^expected &[i32; 1], got [{unknown}; _] + //^^^^expected &[i32; 1], got [{unknown}; {unknown}] // FIXME we only want the outermost error, but this matches the current // behavior of slice patterns let x; [(x,),] = &[(1,),]; // ^^^^expected {unknown}, got ({unknown},) - //^^^^^^^expected &[(i32,); 1], got [{unknown}; _] + //^^^^^^^expected &[(i32,); 1], got [{unknown}; {unknown}] let x; ((x,),) = &((1,),); diff --git a/crates/ide-diagnostics/src/handlers/type_mismatch.rs b/crates/ide-diagnostics/src/handlers/type_mismatch.rs index 442268d135..afb4facb0d 100644 --- a/crates/ide-diagnostics/src/handlers/type_mismatch.rs +++ b/crates/ide-diagnostics/src/handlers/type_mismatch.rs @@ -328,7 +328,7 @@ fn div(x: i32, y: i32) -> Option { } fn main() { run(f()) // FIXME: remove this error - //^^^ error: expected Rate<5>, found Rate<_> + //^^^ error: expected Rate<5>, found Rate<{unknown}> } "#, ); diff --git a/crates/ide/src/hover.rs b/crates/ide/src/hover.rs index 3372d74af6..592fff322e 100644 --- a/crates/ide/src/hover.rs +++ b/crates/ide/src/hover.rs @@ -173,27 +173,17 @@ pub(crate) fn hover_for_definition( Definition::BuiltinType(_) => Some(FamousDefs(sema, sema.scope(node)?.krate())), _ => None, }; - if let Some(markup) = render::definition(sema.db, definition, famous_defs.as_ref(), config) { - let mut res = HoverResult::default(); - res.markup = render::process_markup(sema.db, definition, &markup, config); - if let Some(action) = show_implementations_action(sema.db, definition) { - res.actions.push(action); + render::definition(sema.db, definition, famous_defs.as_ref(), config).map(|markup| { + HoverResult { + markup: render::process_markup(sema.db, definition, &markup, config), + actions: show_implementations_action(sema.db, definition) + .into_iter() + .chain(show_fn_references_action(sema.db, definition)) + .chain(runnable_action(sema, definition, file_id)) + .chain(goto_type_action_for_def(sema.db, definition)) + .collect(), } - - if let Some(action) = show_fn_references_action(sema.db, definition) { - res.actions.push(action); - } - - if let Some(action) = runnable_action(sema, definition, file_id) { - res.actions.push(action); - } - - if let Some(action) = goto_type_action_for_def(sema.db, definition) { - res.actions.push(action); - } - return Some(res); - } - None + }) } fn hover_ranged( diff --git a/lib/la-arena/src/lib.rs b/lib/la-arena/src/lib.rs index 9fe6d60623..0e218806f7 100644 --- a/lib/la-arena/src/lib.rs +++ b/lib/la-arena/src/lib.rs @@ -17,6 +17,12 @@ pub use map::ArenaMap; #[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] pub struct RawIdx(u32); +impl Default for RawIdx { + fn default() -> Self { + Self(u32::MAX) + } +} + impl From for u32 { fn from(raw: RawIdx) -> u32 { raw.0 @@ -47,6 +53,12 @@ pub struct Idx { _ty: PhantomData T>, } +impl Default for Idx { + fn default() -> Self { + Self::from_raw(RawIdx::default()) + } +} + impl Clone for Idx { fn clone(&self) -> Self { *self From 15f73008f87c748d249f8f65e44a10d4d7e65b30 Mon Sep 17 00:00:00 2001 From: Artur Sinila Date: Sun, 17 Jul 2022 14:55:21 +0300 Subject: [PATCH 3/4] refactor: inline some variables --- crates/hir-def/src/body/lower.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/crates/hir-def/src/body/lower.rs b/crates/hir-def/src/body/lower.rs index 37a0940a3f..111460d1a6 100644 --- a/crates/hir-def/src/body/lower.rs +++ b/crates/hir-def/src/body/lower.rs @@ -80,8 +80,7 @@ pub(super) fn lower( params: Option, body: Option, ) -> (Body, BodySourceMap) { - let collector = ExprCollector::new(db, expander); - collector.collect(params, body) + ExprCollector::new(db, expander).collect(params, body) } struct ExprCollector<'a> { @@ -190,8 +189,7 @@ impl<'a> ExprCollector<'a> { } fn collect_expr(&mut self, expr: ast::Expr) -> ExprId { - let expr_id = self.maybe_collect_expr(expr).unwrap_or_else(|| self.missing_expr()); - expr_id + self.maybe_collect_expr(expr).unwrap_or_else(|| self.missing_expr()) } /// Returns `None` if and only if the expression is `#[cfg]`d out. From 83177a7cfea9a99b36733397661b5f79caef42cc Mon Sep 17 00:00:00 2001 From: Artur Sinila Date: Sun, 17 Jul 2022 18:22:11 +0300 Subject: [PATCH 4/4] fix: address suggestions --- crates/hir-def/src/body.rs | 19 ++++++++- crates/hir-def/src/body/lower.rs | 41 +++++++++++-------- crates/hir-def/src/expr.rs | 7 +++- crates/hir-def/src/path/lower.rs | 10 +---- crates/hir-def/src/type_ref.rs | 19 +++++---- crates/hir-ty/src/consteval.rs | 14 ++----- crates/hir-ty/src/infer/pat.rs | 1 + crates/hir-ty/src/interner.rs | 2 +- crates/hir-ty/src/lower.rs | 6 +-- crates/hir-ty/src/tests/simple.rs | 4 +- .../src/handlers/type_mismatch.rs | 2 +- lib/la-arena/src/lib.rs | 12 ------ 12 files changed, 72 insertions(+), 65 deletions(-) diff --git a/crates/hir-def/src/body.rs b/crates/hir-def/src/body.rs index 5c4e7bc338..33851d90a2 100644 --- a/crates/hir-def/src/body.rs +++ b/crates/hir-def/src/body.rs @@ -24,7 +24,7 @@ use syntax::{ast, AstNode, AstPtr, SyntaxNodePtr}; use crate::{ attr::{Attrs, RawAttrs}, db::DefDatabase, - expr::{Expr, ExprId, Label, LabelId, Pat, PatId}, + expr::{dummy_expr_id, Expr, ExprId, Label, LabelId, Pat, PatId}, item_scope::BuiltinShadowMode, macro_id_to_def_id, nameres::DefMap, @@ -238,7 +238,7 @@ pub struct Mark { } /// The body of an item (function, const etc.). -#[derive(Debug, Default, Eq, PartialEq)] +#[derive(Debug, Eq, PartialEq)] pub struct Body { pub exprs: Arena, pub pats: Arena, @@ -389,6 +389,21 @@ impl Body { } } +impl Default for Body { + fn default() -> Self { + Self { + body_expr: dummy_expr_id(), + exprs: Default::default(), + pats: Default::default(), + or_pats: Default::default(), + labels: Default::default(), + params: Default::default(), + block_scopes: Default::default(), + _c: Default::default(), + } + } +} + impl Index for Body { type Output = Expr; diff --git a/crates/hir-def/src/body/lower.rs b/crates/hir-def/src/body/lower.rs index 111460d1a6..049afa8227 100644 --- a/crates/hir-def/src/body/lower.rs +++ b/crates/hir-def/src/body/lower.rs @@ -1,7 +1,7 @@ //! Transforms `ast::Expr` into an equivalent `hir_def::expr::Expr` //! representation. -use std::{collections::HashMap, mem, sync::Arc}; +use std::{mem, sync::Arc}; use either::Either; use hir_expand::{ @@ -10,6 +10,8 @@ use hir_expand::{ name::{name, AsName, Name}, ExpandError, HirFileId, InFile, }; +use la_arena::Arena; +use profile::Count; use rustc_hash::FxHashMap; use syntax::{ ast::{ @@ -26,8 +28,8 @@ use crate::{ builtin_type::{BuiltinFloat, BuiltinInt, BuiltinUint}, db::DefDatabase, expr::{ - Array, BindingAnnotation, Expr, ExprId, FloatTypeWrapper, Label, LabelId, Literal, - MatchArm, Pat, PatId, RecordFieldPat, RecordLitField, Statement, + dummy_expr_id, Array, BindingAnnotation, Expr, ExprId, FloatTypeWrapper, Label, LabelId, + Literal, MatchArm, Pat, PatId, RecordFieldPat, RecordLitField, Statement, }, intern::Interned, item_scope::BuiltinShadowMode, @@ -80,7 +82,24 @@ pub(super) fn lower( params: Option, body: Option, ) -> (Body, BodySourceMap) { - ExprCollector::new(db, expander).collect(params, body) + ExprCollector { + db, + source_map: BodySourceMap::default(), + body: Body { + exprs: Arena::default(), + pats: Arena::default(), + labels: Arena::default(), + params: Vec::new(), + body_expr: dummy_expr_id(), + block_scopes: Vec::new(), + _c: Count::new(), + or_pats: Default::default(), + }, + expander, + name_to_pat_grouping: Default::default(), + is_lowering_inside_or_pat: false, + } + .collect(params, body) } struct ExprCollector<'a> { @@ -93,18 +112,7 @@ struct ExprCollector<'a> { is_lowering_inside_or_pat: bool, } -impl<'a> ExprCollector<'a> { - pub(crate) fn new(db: &'a dyn DefDatabase, expander: Expander) -> Self { - Self { - db, - expander, - body: Body::default(), - source_map: BodySourceMap::default(), - name_to_pat_grouping: HashMap::default(), - is_lowering_inside_or_pat: false, - } - } - +impl ExprCollector<'_> { fn collect( mut self, param_list: Option, @@ -681,6 +689,7 @@ impl<'a> ExprCollector<'a> { }; let prev_def_map = mem::replace(&mut self.expander.def_map, def_map); let prev_local_module = mem::replace(&mut self.expander.module, module); + let mut statements: Vec<_> = block.statements().filter_map(|s| self.collect_stmt(s)).collect(); let tail = block.tail_expr().and_then(|e| self.maybe_collect_expr(e)); diff --git a/crates/hir-def/src/expr.rs b/crates/hir-def/src/expr.rs index c25aa0bcaf..a991365d6b 100644 --- a/crates/hir-def/src/expr.rs +++ b/crates/hir-def/src/expr.rs @@ -13,7 +13,7 @@ //! See also a neighboring `body` module. use hir_expand::name::Name; -use la_arena::Idx; +use la_arena::{Idx, RawIdx}; use crate::{ builtin_type::{BuiltinFloat, BuiltinInt, BuiltinUint}, @@ -27,6 +27,11 @@ pub use syntax::ast::{ArithOp, BinaryOp, CmpOp, LogicOp, Ordering, RangeOp, Unar pub type ExprId = Idx; +/// FIXME: this is a hacky function which should be removed +pub(crate) fn dummy_expr_id() -> ExprId { + ExprId::from_raw(RawIdx::from(u32::MAX)) +} + pub type PatId = Idx; #[derive(Debug, Clone, Eq, PartialEq)] diff --git a/crates/hir-def/src/path/lower.rs b/crates/hir-def/src/path/lower.rs index 6a15b1c085..b6a24cd4ab 100644 --- a/crates/hir-def/src/path/lower.rs +++ b/crates/hir-def/src/path/lower.rs @@ -1,9 +1,6 @@ //! Transforms syntax into `Path` objects, ideally with accounting for hygiene -use crate::{ - intern::Interned, - type_ref::{ConstScalar, ConstScalarOrPath}, -}; +use crate::{intern::Interned, type_ref::ConstScalarOrPath}; use either::Either; use hir_expand::name::{name, AsName}; @@ -184,10 +181,7 @@ pub(super) fn lower_generic_args( } } ast::GenericArg::ConstArg(arg) => { - let arg = arg.expr().map_or( - ConstScalarOrPath::Scalar(ConstScalar::Unknown), - ConstScalarOrPath::from_expr, - ); + let arg = ConstScalarOrPath::from_expr_opt(arg.expr()); args.push(GenericArg::Const(arg)) } } diff --git a/crates/hir-def/src/type_ref.rs b/crates/hir-def/src/type_ref.rs index 1916a6b3c5..867d82f45a 100644 --- a/crates/hir-def/src/type_ref.rs +++ b/crates/hir-def/src/type_ref.rs @@ -1,6 +1,8 @@ //! HIR for references to types. Paths in these are not yet resolved. They can //! be directly created from an ast::TypeRef, without further queries. +use std::fmt::Write; + use hir_expand::{ name::{AsName, Name}, AstId, InFile, @@ -182,11 +184,7 @@ impl TypeRef { // `hir_def::body::lower` to lower this into an `Expr` and then evaluate it at the // `hir_ty` level, which would allow knowing the type of: // let v: [u8; 2 + 2] = [0u8; 4]; - let len = inner.expr().map_or( - ConstScalarOrPath::Scalar(ConstScalar::Unknown), - ConstScalarOrPath::from_expr, - ); - + let len = ConstScalarOrPath::from_expr_opt(inner.expr()); TypeRef::Array(Box::new(TypeRef::from_ast_opt(ctx, inner.ty())), len) } ast::Type::SliceType(inner) => { @@ -394,9 +392,16 @@ impl std::fmt::Display for ConstScalarOrPath { } impl ConstScalarOrPath { + pub(crate) fn from_expr_opt(expr: Option) -> Self { + match expr { + Some(x) => Self::from_expr(x), + None => Self::Scalar(ConstScalar::Unknown), + } + } + // FIXME: as per the comments on `TypeRef::Array`, this evaluation should not happen at this // parse stage. - pub(crate) fn from_expr(expr: ast::Expr) -> Self { + fn from_expr(expr: ast::Expr) -> Self { match expr { ast::Expr::PathExpr(p) => { match p.path().and_then(|x| x.segment()).and_then(|x| x.name_ref()) { @@ -480,7 +485,7 @@ impl std::fmt::Display for ConstScalar { ConstScalar::UInt(num) => num.fmt(f), ConstScalar::Bool(flag) => flag.fmt(f), ConstScalar::Char(c) => write!(f, "'{c}'"), - ConstScalar::Unknown => f.write_str("{unknown}"), + ConstScalar::Unknown => f.write_char('_'), } } } diff --git a/crates/hir-ty/src/consteval.rs b/crates/hir-ty/src/consteval.rs index 61183be04f..0495a4e64c 100644 --- a/crates/hir-ty/src/consteval.rs +++ b/crates/hir-ty/src/consteval.rs @@ -395,22 +395,14 @@ pub fn unknown_const_as_generic(ty: Ty) -> GenericArg { } /// Interns a constant scalar with the given type -pub fn intern_const_scalar_with_type(value: ConstScalar, ty: Ty) -> Const { +pub fn intern_const_scalar(value: ConstScalar, ty: Ty) -> Const { ConstData { ty, value: ConstValue::Concrete(chalk_ir::ConcreteConst { interned: value }) } .intern(Interner) } /// Interns a possibly-unknown target usize pub fn usize_const(value: Option) -> Const { - intern_const_scalar_with_type( - value.map(ConstScalar::UInt).unwrap_or(ConstScalar::Unknown), - TyBuilder::usize(), - ) -} - -/// Interns a constant scalar with the default type -pub fn intern_const_scalar(value: ConstScalar) -> Const { - intern_const_scalar_with_type(value, TyBuilder::builtin(value.builtin_type())) + intern_const_scalar(value.map_or(ConstScalar::Unknown, ConstScalar::UInt), TyBuilder::usize()) } pub(crate) fn const_eval_recover( @@ -470,7 +462,7 @@ pub(crate) fn eval_to_const<'a>( Ok(ComputedExpr::Literal(literal)) => literal.into(), _ => ConstScalar::Unknown, }; - intern_const_scalar_with_type(const_scalar, TyBuilder::usize()) + intern_const_scalar(const_scalar, TyBuilder::usize()) } #[cfg(test)] diff --git a/crates/hir-ty/src/infer/pat.rs b/crates/hir-ty/src/infer/pat.rs index 327e463eec..5e7320a5dd 100644 --- a/crates/hir-ty/src/infer/pat.rs +++ b/crates/hir-ty/src/infer/pat.rs @@ -273,6 +273,7 @@ impl<'a> InferenceContext<'a> { elem_ty.clone(), intern_const_scalar( len.map_or(ConstScalar::Unknown, |len| ConstScalar::UInt(len)), + TyBuilder::usize(), ), ) } diff --git a/crates/hir-ty/src/interner.rs b/crates/hir-ty/src/interner.rs index 479521e8ba..c34c4b8a7c 100644 --- a/crates/hir-ty/src/interner.rs +++ b/crates/hir-ty/src/interner.rs @@ -257,7 +257,7 @@ impl chalk_ir::interner::Interner for Interner { c1: &Self::InternedConcreteConst, c2: &Self::InternedConcreteConst, ) -> bool { - c1 == c2 + (c1 == &ConstScalar::Unknown) || (c2 == &ConstScalar::Unknown) || (c1 == c2) } fn intern_generic_arg( diff --git a/crates/hir-ty/src/lower.rs b/crates/hir-ty/src/lower.rs index f62972b1eb..e86b52c987 100644 --- a/crates/hir-ty/src/lower.rs +++ b/crates/hir-ty/src/lower.rs @@ -44,9 +44,7 @@ use syntax::{ast, SmolStr}; use crate::{ all_super_traits, - consteval::{ - intern_const_scalar_with_type, path_to_const, unknown_const, unknown_const_as_generic, - }, + consteval::{intern_const_scalar, path_to_const, unknown_const, unknown_const_as_generic}, db::HirDatabase, make_binders, mapping::ToChalk, @@ -1744,7 +1742,7 @@ pub(crate) fn const_or_path_to_chalk( debruijn: DebruijnIndex, ) -> Const { match value { - ConstScalarOrPath::Scalar(s) => intern_const_scalar_with_type(s.clone(), expected_ty), + ConstScalarOrPath::Scalar(s) => intern_const_scalar(s.clone(), expected_ty), ConstScalarOrPath::Path(n) => { let path = ModPath::from_segments(PathKind::Plain, Some(n.clone())); path_to_const(db, resolver, &path, mode, args, debruijn) diff --git a/crates/hir-ty/src/tests/simple.rs b/crates/hir-ty/src/tests/simple.rs index 4d4795e933..5b08f55210 100644 --- a/crates/hir-ty/src/tests/simple.rs +++ b/crates/hir-ty/src/tests/simple.rs @@ -3011,14 +3011,14 @@ struct TS(usize); fn main() { let x; [x,] = &[1,]; - //^^^^expected &[i32; 1], got [{unknown}; {unknown}] + //^^^^expected &[i32; 1], got [{unknown}; _] // FIXME we only want the outermost error, but this matches the current // behavior of slice patterns let x; [(x,),] = &[(1,),]; // ^^^^expected {unknown}, got ({unknown},) - //^^^^^^^expected &[(i32,); 1], got [{unknown}; {unknown}] + //^^^^^^^expected &[(i32,); 1], got [{unknown}; _] let x; ((x,),) = &((1,),); diff --git a/crates/ide-diagnostics/src/handlers/type_mismatch.rs b/crates/ide-diagnostics/src/handlers/type_mismatch.rs index afb4facb0d..442268d135 100644 --- a/crates/ide-diagnostics/src/handlers/type_mismatch.rs +++ b/crates/ide-diagnostics/src/handlers/type_mismatch.rs @@ -328,7 +328,7 @@ fn div(x: i32, y: i32) -> Option { } fn main() { run(f()) // FIXME: remove this error - //^^^ error: expected Rate<5>, found Rate<{unknown}> + //^^^ error: expected Rate<5>, found Rate<_> } "#, ); diff --git a/lib/la-arena/src/lib.rs b/lib/la-arena/src/lib.rs index 0e218806f7..9fe6d60623 100644 --- a/lib/la-arena/src/lib.rs +++ b/lib/la-arena/src/lib.rs @@ -17,12 +17,6 @@ pub use map::ArenaMap; #[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] pub struct RawIdx(u32); -impl Default for RawIdx { - fn default() -> Self { - Self(u32::MAX) - } -} - impl From for u32 { fn from(raw: RawIdx) -> u32 { raw.0 @@ -53,12 +47,6 @@ pub struct Idx { _ty: PhantomData T>, } -impl Default for Idx { - fn default() -> Self { - Self::from_raw(RawIdx::default()) - } -} - impl Clone for Idx { fn clone(&self) -> Self { *self