Merge pull request #18995 from alibektas/12210

fix: Lower range pattern bounds to expressions

crates/hir-def/src/expr_store.rs

@@ -112,9 +112,9 @@ pub struct ExpressionStoreSourceMap {
     // AST expressions can create patterns in destructuring assignments. Therefore, `ExprSource` can also map
     // to `PatId`, and `PatId` can also map to `ExprSource` (the other way around is unaffected).
     expr_map: FxHashMap<ExprSource, ExprOrPatId>,
-    expr_map_back: ArenaMap<ExprId, ExprSource>,
+    expr_map_back: ArenaMap<ExprId, ExprOrPatSource>,
 
-    pat_map: FxHashMap<PatSource, PatId>,
+    pat_map: FxHashMap<PatSource, ExprOrPatId>,
     pat_map_back: ArenaMap<PatId, ExprOrPatSource>,
 
     label_map: FxHashMap<LabelSource, LabelId>,
@@ -606,12 +606,12 @@ impl Index<TypeRefId> for ExpressionStore {
 impl ExpressionStoreSourceMap {
     pub fn expr_or_pat_syntax(&self, id: ExprOrPatId) -> Result<ExprOrPatSource, SyntheticSyntax> {
         match id {
-            ExprOrPatId::ExprId(id) => self.expr_syntax(id).map(|it| it.map(AstPtr::wrap_left)),
+            ExprOrPatId::ExprId(id) => self.expr_syntax(id),
             ExprOrPatId::PatId(id) => self.pat_syntax(id),
         }
     }
 
-    pub fn expr_syntax(&self, expr: ExprId) -> Result<ExprSource, SyntheticSyntax> {
+    pub fn expr_syntax(&self, expr: ExprId) -> Result<ExprOrPatSource, SyntheticSyntax> {
         self.expr_map_back.get(expr).cloned().ok_or(SyntheticSyntax)
     }
 
@@ -633,7 +633,7 @@ impl ExpressionStoreSourceMap {
         self.pat_map_back.get(pat).cloned().ok_or(SyntheticSyntax)
     }
 
-    pub fn node_pat(&self, node: InFile<&ast::Pat>) -> Option<PatId> {
+    pub fn node_pat(&self, node: InFile<&ast::Pat>) -> Option<ExprOrPatId> {
         self.pat_map.get(&node.map(AstPtr::new)).cloned()
     }
 

crates/hir-def/src/expr_store/lower.rs

@@ -44,8 +44,8 @@ use crate::{
             FormatPlaceholder, FormatSign, FormatTrait,
         },
         Array, Binding, BindingAnnotation, BindingId, BindingProblems, CaptureBy, ClosureKind,
-        Expr, ExprId, Item, Label, LabelId, Literal, LiteralOrConst, MatchArm, Movability,
-        OffsetOf, Pat, PatId, RecordFieldPat, RecordLitField, Statement,
+        Expr, ExprId, Item, Label, LabelId, Literal, MatchArm, Movability, OffsetOf, Pat, PatId,
+        RecordFieldPat, RecordLitField, Statement,
     },
     item_scope::BuiltinShadowMode,
     lang_item::LangItem,
@@ -1784,23 +1784,33 @@ impl ExprCollector<'_> {
                         self.collect_macro_call(call, macro_ptr, true, |this, expanded_pat| {
                             this.collect_pat_opt(expanded_pat, binding_list)
                         });
-                    self.source_map.pat_map.insert(src, pat);
+                    self.source_map.pat_map.insert(src, pat.into());
                     return pat;
                 }
                 None => Pat::Missing,
             },
-            // FIXME: implement in a way that also builds source map and calculates assoc resolutions in type inference.
             ast::Pat::RangePat(p) => {
-                let mut range_part_lower = |p: Option<ast::Pat>| {
-                    p.and_then(|it| match &it {
-                        ast::Pat::LiteralPat(it) => {
-                            Some(Box::new(LiteralOrConst::Literal(pat_literal_to_hir(it)?.0)))
+                let mut range_part_lower = |p: Option<ast::Pat>| -> Option<ExprId> {
+                    p.and_then(|it| {
+                        let ptr = PatPtr::new(&it);
+                        match &it {
+                            ast::Pat::LiteralPat(it) => Some(self.alloc_expr_from_pat(
+                                Expr::Literal(pat_literal_to_hir(it)?.0),
+                                ptr,
+                            )),
+                            ast::Pat::IdentPat(ident) if ident.is_simple_ident() => ident
+                                .name()
+                                .map(|name| name.as_name())
+                                .map(Path::from)
+                                .map(|path| self.alloc_expr_from_pat(Expr::Path(path), ptr)),
+                            ast::Pat::PathPat(p) => p
+                                .path()
+                                .and_then(|path| self.parse_path(path))
+                                .map(|parsed| self.alloc_expr_from_pat(Expr::Path(parsed), ptr)),
+                            // We only need to handle literal, ident (if bare) and path patterns here,
+                            // as any other pattern as a range pattern operand is semantically invalid.
+                            _ => None,
                         }
-                        pat @ (ast::Pat::IdentPat(_) | ast::Pat::PathPat(_)) => {
-                            let subpat = self.collect_pat(pat.clone(), binding_list);
-                            Some(Box::new(LiteralOrConst::Const(subpat)))
-                        }
-                        _ => None,
                     })
                 };
                 let start = range_part_lower(p.start());
@@ -1863,7 +1873,7 @@ impl ExprCollector<'_> {
                             }
                         });
                     if let Some(pat) = pat.left() {
-                        self.source_map.pat_map.insert(src, pat);
+                        self.source_map.pat_map.insert(src, pat.into());
                     }
                     pat
                 }
@@ -2490,7 +2500,7 @@ impl ExprCollector<'_> {
     fn alloc_expr(&mut self, expr: Expr, ptr: ExprPtr) -> ExprId {
         let src = self.expander.in_file(ptr);
         let id = self.store.exprs.alloc(expr);
-        self.source_map.expr_map_back.insert(id, src);
+        self.source_map.expr_map_back.insert(id, src.map(AstPtr::wrap_left));
         self.source_map.expr_map.insert(src, id.into());
         id
     }
@@ -2502,7 +2512,7 @@ impl ExprCollector<'_> {
     fn alloc_expr_desugared_with_ptr(&mut self, expr: Expr, ptr: ExprPtr) -> ExprId {
         let src = self.expander.in_file(ptr);
         let id = self.store.exprs.alloc(expr);
-        self.source_map.expr_map_back.insert(id, src);
+        self.source_map.expr_map_back.insert(id, src.map(AstPtr::wrap_left));
         // We intentionally don't fill this as it could overwrite a non-desugared entry
         // self.source_map.expr_map.insert(src, id);
         id
@@ -2526,11 +2536,20 @@ impl ExprCollector<'_> {
         self.source_map.pat_map_back.insert(id, src.map(AstPtr::wrap_left));
         id
     }
+
+    fn alloc_expr_from_pat(&mut self, expr: Expr, ptr: PatPtr) -> ExprId {
+        let src = self.expander.in_file(ptr);
+        let id = self.store.exprs.alloc(expr);
+        self.source_map.pat_map.insert(src, id.into());
+        self.source_map.expr_map_back.insert(id, src.map(AstPtr::wrap_right));
+        id
+    }
+
     fn alloc_pat(&mut self, pat: Pat, ptr: PatPtr) -> PatId {
         let src = self.expander.in_file(ptr);
         let id = self.store.pats.alloc(pat);
         self.source_map.pat_map_back.insert(id, src.map(AstPtr::wrap_right));
-        self.source_map.pat_map.insert(src, id);
+        self.source_map.pat_map.insert(src, id.into());
         id
     }
     // FIXME: desugared pats don't have ptr, that's wrong and should be fixed somehow.

crates/hir-def/src/expr_store/pretty.rs

@@ -6,10 +6,7 @@ use itertools::Itertools;
 use span::Edition;
 
 use crate::{
-    hir::{
-        Array, BindingAnnotation, CaptureBy, ClosureKind, Literal, LiteralOrConst, Movability,
-        Statement,
-    },
+    hir::{Array, BindingAnnotation, CaptureBy, ClosureKind, Literal, Movability, Statement},
     pretty::{print_generic_args, print_path, print_type_ref},
 };
 
@@ -656,11 +653,11 @@ impl Printer<'_> {
             }
             Pat::Range { start, end } => {
                 if let Some(start) = start {
-                    self.print_literal_or_const(start);
+                    self.print_expr(*start);
                 }
                 w!(self, "..=");
                 if let Some(end) = end {
-                    self.print_literal_or_const(end);
+                    self.print_expr(*end);
                 }
             }
             Pat::Slice { prefix, slice, suffix } => {
@@ -757,13 +754,6 @@ impl Printer<'_> {
         }
     }
 
-    fn print_literal_or_const(&mut self, literal_or_const: &LiteralOrConst) {
-        match literal_or_const {
-            LiteralOrConst::Literal(l) => self.print_literal(l),
-            LiteralOrConst::Const(c) => self.print_pat(*c),
-        }
-    }
-
     fn print_literal(&mut self, literal: &Literal) {
         match literal {
             Literal::String(it) => w!(self, "{:?}", it),

crates/hir-def/src/expr_store/tests.rs

@@ -1,11 +1,10 @@
 mod block;
 
+use crate::{hir::MatchArm, test_db::TestDB, ModuleDefId};
 use expect_test::{expect, Expect};
 use la_arena::RawIdx;
 use test_fixture::WithFixture;
 
-use crate::{test_db::TestDB, ModuleDefId};
-
 use super::*;
 
 fn lower(#[rust_analyzer::rust_fixture] ra_fixture: &str) -> (TestDB, Arc<Body>, DefWithBodyId) {
@@ -460,3 +459,45 @@ async fn foo(a: (), b: i32) -> u32 {
     expect!["fn foo(<28>: (), <20>: i32) -> impl ::core::future::Future::<Output = u32> <20>"]
         .assert_eq(&printed);
 }
+
+#[test]
+fn range_bounds_are_hir_exprs() {
+    let (_, body, _) = lower(
+        r#"
+pub const L: i32 = 6;
+mod x {
+    pub const R: i32 = 100;
+}
+const fn f(x: i32) -> i32 {
+    match x {
+        -1..=5 => x * 10,
+        L..=x::R => x * 100,
+        _ => x,
+    }
+}"#,
+    );
+
+    let mtch_arms = body
+        .exprs
+        .iter()
+        .find_map(|(_, expr)| {
+            if let Expr::Match { arms, .. } = expr {
+                return Some(arms);
+            }
+
+            None
+        })
+        .unwrap();
+
+    let MatchArm { pat, .. } = mtch_arms[1];
+    match body.pats[pat] {
+        Pat::Range { start, end } => {
+            let hir_start = &body.exprs[start.unwrap()];
+            let hir_end = &body.exprs[end.unwrap()];
+
+            assert!(matches!(hir_start, Expr::Path { .. }));
+            assert!(matches!(hir_end, Expr::Path { .. }));
+        }
+        _ => {}
+    }
+}

crates/hir-def/src/hir.rs

@@ -55,12 +55,20 @@ impl ExprOrPatId {
         }
     }
 
+    pub fn is_expr(&self) -> bool {
+        matches!(self, Self::ExprId(_))
+    }
+
     pub fn as_pat(self) -> Option<PatId> {
         match self {
             Self::PatId(v) => Some(v),
             _ => None,
         }
     }
+
+    pub fn is_pat(&self) -> bool {
+        matches!(self, Self::PatId(_))
+    }
 }
 stdx::impl_from!(ExprId, PatId for ExprOrPatId);
 
@@ -571,8 +579,8 @@ pub enum Pat {
         ellipsis: bool,
     },
     Range {
-        start: Option<Box<LiteralOrConst>>,
-        end: Option<Box<LiteralOrConst>>,
+        start: Option<ExprId>,
+        end: Option<ExprId>,
     },
     Slice {
         prefix: Box<[PatId]>,