fix: address suggestions

rust-lang · Jul 17, 2022 · 83177a7 · 83177a7
1 parent 15f7300
commit 83177a7
Show file tree

Hide file tree

Showing 12 changed files with 72 additions and 65 deletions.
diff --git 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<Expr>,
     pub pats: Arena<Pat>,
@@ -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<ExprId> for Body {
     type Output = Expr;
 

diff --git 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<ast::ParamList>,
     body: Option<ast::Expr>,
 ) -> (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<ast::ParamList>,
@@ -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
@@ -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<Expr>;
 
+/// 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<Pat>;
 
 #[derive(Debug, Clone, Eq, PartialEq)]

diff --git 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
@@ -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<ast::Expr>) -> 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
@@ -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<u128>) -> 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -328,7 +328,7 @@ fn div(x: i32, y: i32) -> Option<i32> {
             }
             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
@@ -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<RawIdx> for u32 {
     fn from(raw: RawIdx) -> u32 {
         raw.0
@@ -53,12 +47,6 @@ pub struct Idx<T> {
     _ty: PhantomData<fn() -> T>,
 }
 
-impl<T> Default for Idx<T> {
-    fn default() -> Self {
-        Self::from_raw(RawIdx::default())
-    }
-}
-
 impl<T> Clone for Idx<T> {
     fn clone(&self) -> Self {
         *self