Skip to content

Commit 444c2f9

Browse files
authored
Merge pull request rust-lang#18995 from alibektas/12210
fix: Lower range pattern bounds to expressions
2 parents ad8c6c6 + bc5f61c commit 444c2f9

File tree

16 files changed

+187
-117
lines changed

16 files changed

+187
-117
lines changed

src/tools/rust-analyzer/crates/hir-def/src/expr_store.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -112,9 +112,9 @@ pub struct ExpressionStoreSourceMap {
112112
// AST expressions can create patterns in destructuring assignments. Therefore, `ExprSource` can also map
113113
// to `PatId`, and `PatId` can also map to `ExprSource` (the other way around is unaffected).
114114
expr_map: FxHashMap<ExprSource, ExprOrPatId>,
115-
expr_map_back: ArenaMap<ExprId, ExprSource>,
115+
expr_map_back: ArenaMap<ExprId, ExprOrPatSource>,
116116

117-
pat_map: FxHashMap<PatSource, PatId>,
117+
pat_map: FxHashMap<PatSource, ExprOrPatId>,
118118
pat_map_back: ArenaMap<PatId, ExprOrPatSource>,
119119

120120
label_map: FxHashMap<LabelSource, LabelId>,
@@ -606,12 +606,12 @@ impl Index<TypeRefId> for ExpressionStore {
606606
impl ExpressionStoreSourceMap {
607607
pub fn expr_or_pat_syntax(&self, id: ExprOrPatId) -> Result<ExprOrPatSource, SyntheticSyntax> {
608608
match id {
609-
ExprOrPatId::ExprId(id) => self.expr_syntax(id).map(|it| it.map(AstPtr::wrap_left)),
609+
ExprOrPatId::ExprId(id) => self.expr_syntax(id),
610610
ExprOrPatId::PatId(id) => self.pat_syntax(id),
611611
}
612612
}
613613

614-
pub fn expr_syntax(&self, expr: ExprId) -> Result<ExprSource, SyntheticSyntax> {
614+
pub fn expr_syntax(&self, expr: ExprId) -> Result<ExprOrPatSource, SyntheticSyntax> {
615615
self.expr_map_back.get(expr).cloned().ok_or(SyntheticSyntax)
616616
}
617617

@@ -633,7 +633,7 @@ impl ExpressionStoreSourceMap {
633633
self.pat_map_back.get(pat).cloned().ok_or(SyntheticSyntax)
634634
}
635635

636-
pub fn node_pat(&self, node: InFile<&ast::Pat>) -> Option<PatId> {
636+
pub fn node_pat(&self, node: InFile<&ast::Pat>) -> Option<ExprOrPatId> {
637637
self.pat_map.get(&node.map(AstPtr::new)).cloned()
638638
}
639639

src/tools/rust-analyzer/crates/hir-def/src/expr_store/lower.rs

Lines changed: 36 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,8 @@ use crate::{
4444
FormatPlaceholder, FormatSign, FormatTrait,
4545
},
4646
Array, Binding, BindingAnnotation, BindingId, BindingProblems, CaptureBy, ClosureKind,
47-
Expr, ExprId, Item, Label, LabelId, Literal, LiteralOrConst, MatchArm, Movability,
48-
OffsetOf, Pat, PatId, RecordFieldPat, RecordLitField, Statement,
47+
Expr, ExprId, Item, Label, LabelId, Literal, MatchArm, Movability, OffsetOf, Pat, PatId,
48+
RecordFieldPat, RecordLitField, Statement,
4949
},
5050
item_scope::BuiltinShadowMode,
5151
lang_item::LangItem,
@@ -1784,23 +1784,33 @@ impl ExprCollector<'_> {
17841784
self.collect_macro_call(call, macro_ptr, true, |this, expanded_pat| {
17851785
this.collect_pat_opt(expanded_pat, binding_list)
17861786
});
1787-
self.source_map.pat_map.insert(src, pat);
1787+
self.source_map.pat_map.insert(src, pat.into());
17881788
return pat;
17891789
}
17901790
None => Pat::Missing,
17911791
},
1792-
// FIXME: implement in a way that also builds source map and calculates assoc resolutions in type inference.
17931792
ast::Pat::RangePat(p) => {
1794-
let mut range_part_lower = |p: Option<ast::Pat>| {
1795-
p.and_then(|it| match &it {
1796-
ast::Pat::LiteralPat(it) => {
1797-
Some(Box::new(LiteralOrConst::Literal(pat_literal_to_hir(it)?.0)))
1793+
let mut range_part_lower = |p: Option<ast::Pat>| -> Option<ExprId> {
1794+
p.and_then(|it| {
1795+
let ptr = PatPtr::new(&it);
1796+
match &it {
1797+
ast::Pat::LiteralPat(it) => Some(self.alloc_expr_from_pat(
1798+
Expr::Literal(pat_literal_to_hir(it)?.0),
1799+
ptr,
1800+
)),
1801+
ast::Pat::IdentPat(ident) if ident.is_simple_ident() => ident
1802+
.name()
1803+
.map(|name| name.as_name())
1804+
.map(Path::from)
1805+
.map(|path| self.alloc_expr_from_pat(Expr::Path(path), ptr)),
1806+
ast::Pat::PathPat(p) => p
1807+
.path()
1808+
.and_then(|path| self.parse_path(path))
1809+
.map(|parsed| self.alloc_expr_from_pat(Expr::Path(parsed), ptr)),
1810+
// We only need to handle literal, ident (if bare) and path patterns here,
1811+
// as any other pattern as a range pattern operand is semantically invalid.
1812+
_ => None,
17981813
}
1799-
pat @ (ast::Pat::IdentPat(_) | ast::Pat::PathPat(_)) => {
1800-
let subpat = self.collect_pat(pat.clone(), binding_list);
1801-
Some(Box::new(LiteralOrConst::Const(subpat)))
1802-
}
1803-
_ => None,
18041814
})
18051815
};
18061816
let start = range_part_lower(p.start());
@@ -1863,7 +1873,7 @@ impl ExprCollector<'_> {
18631873
}
18641874
});
18651875
if let Some(pat) = pat.left() {
1866-
self.source_map.pat_map.insert(src, pat);
1876+
self.source_map.pat_map.insert(src, pat.into());
18671877
}
18681878
pat
18691879
}
@@ -2490,7 +2500,7 @@ impl ExprCollector<'_> {
24902500
fn alloc_expr(&mut self, expr: Expr, ptr: ExprPtr) -> ExprId {
24912501
let src = self.expander.in_file(ptr);
24922502
let id = self.store.exprs.alloc(expr);
2493-
self.source_map.expr_map_back.insert(id, src);
2503+
self.source_map.expr_map_back.insert(id, src.map(AstPtr::wrap_left));
24942504
self.source_map.expr_map.insert(src, id.into());
24952505
id
24962506
}
@@ -2502,7 +2512,7 @@ impl ExprCollector<'_> {
25022512
fn alloc_expr_desugared_with_ptr(&mut self, expr: Expr, ptr: ExprPtr) -> ExprId {
25032513
let src = self.expander.in_file(ptr);
25042514
let id = self.store.exprs.alloc(expr);
2505-
self.source_map.expr_map_back.insert(id, src);
2515+
self.source_map.expr_map_back.insert(id, src.map(AstPtr::wrap_left));
25062516
// We intentionally don't fill this as it could overwrite a non-desugared entry
25072517
// self.source_map.expr_map.insert(src, id);
25082518
id
@@ -2526,11 +2536,20 @@ impl ExprCollector<'_> {
25262536
self.source_map.pat_map_back.insert(id, src.map(AstPtr::wrap_left));
25272537
id
25282538
}
2539+
2540+
fn alloc_expr_from_pat(&mut self, expr: Expr, ptr: PatPtr) -> ExprId {
2541+
let src = self.expander.in_file(ptr);
2542+
let id = self.store.exprs.alloc(expr);
2543+
self.source_map.pat_map.insert(src, id.into());
2544+
self.source_map.expr_map_back.insert(id, src.map(AstPtr::wrap_right));
2545+
id
2546+
}
2547+
25292548
fn alloc_pat(&mut self, pat: Pat, ptr: PatPtr) -> PatId {
25302549
let src = self.expander.in_file(ptr);
25312550
let id = self.store.pats.alloc(pat);
25322551
self.source_map.pat_map_back.insert(id, src.map(AstPtr::wrap_right));
2533-
self.source_map.pat_map.insert(src, id);
2552+
self.source_map.pat_map.insert(src, id.into());
25342553
id
25352554
}
25362555
// FIXME: desugared pats don't have ptr, that's wrong and should be fixed somehow.

src/tools/rust-analyzer/crates/hir-def/src/expr_store/pretty.rs

Lines changed: 3 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,7 @@ use itertools::Itertools;
66
use span::Edition;
77

88
use crate::{
9-
hir::{
10-
Array, BindingAnnotation, CaptureBy, ClosureKind, Literal, LiteralOrConst, Movability,
11-
Statement,
12-
},
9+
hir::{Array, BindingAnnotation, CaptureBy, ClosureKind, Literal, Movability, Statement},
1310
pretty::{print_generic_args, print_path, print_type_ref},
1411
};
1512

@@ -656,11 +653,11 @@ impl Printer<'_> {
656653
}
657654
Pat::Range { start, end } => {
658655
if let Some(start) = start {
659-
self.print_literal_or_const(start);
656+
self.print_expr(*start);
660657
}
661658
w!(self, "..=");
662659
if let Some(end) = end {
663-
self.print_literal_or_const(end);
660+
self.print_expr(*end);
664661
}
665662
}
666663
Pat::Slice { prefix, slice, suffix } => {
@@ -757,13 +754,6 @@ impl Printer<'_> {
757754
}
758755
}
759756

760-
fn print_literal_or_const(&mut self, literal_or_const: &LiteralOrConst) {
761-
match literal_or_const {
762-
LiteralOrConst::Literal(l) => self.print_literal(l),
763-
LiteralOrConst::Const(c) => self.print_pat(*c),
764-
}
765-
}
766-
767757
fn print_literal(&mut self, literal: &Literal) {
768758
match literal {
769759
Literal::String(it) => w!(self, "{:?}", it),

src/tools/rust-analyzer/crates/hir-def/src/expr_store/tests.rs

Lines changed: 43 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,10 @@
11
mod block;
22

3+
use crate::{hir::MatchArm, test_db::TestDB, ModuleDefId};
34
use expect_test::{expect, Expect};
45
use la_arena::RawIdx;
56
use test_fixture::WithFixture;
67

7-
use crate::{test_db::TestDB, ModuleDefId};
8-
98
use super::*;
109

1110
fn lower(#[rust_analyzer::rust_fixture] ra_fixture: &str) -> (TestDB, Arc<Body>, DefWithBodyId) {
@@ -460,3 +459,45 @@ async fn foo(a: (), b: i32) -> u32 {
460459
expect!["fn foo(�: (), �: i32) -> impl ::core::future::Future::<Output = u32> �"]
461460
.assert_eq(&printed);
462461
}
462+
463+
#[test]
464+
fn range_bounds_are_hir_exprs() {
465+
let (_, body, _) = lower(
466+
r#"
467+
pub const L: i32 = 6;
468+
mod x {
469+
pub const R: i32 = 100;
470+
}
471+
const fn f(x: i32) -> i32 {
472+
match x {
473+
-1..=5 => x * 10,
474+
L..=x::R => x * 100,
475+
_ => x,
476+
}
477+
}"#,
478+
);
479+
480+
let mtch_arms = body
481+
.exprs
482+
.iter()
483+
.find_map(|(_, expr)| {
484+
if let Expr::Match { arms, .. } = expr {
485+
return Some(arms);
486+
}
487+
488+
None
489+
})
490+
.unwrap();
491+
492+
let MatchArm { pat, .. } = mtch_arms[1];
493+
match body.pats[pat] {
494+
Pat::Range { start, end } => {
495+
let hir_start = &body.exprs[start.unwrap()];
496+
let hir_end = &body.exprs[end.unwrap()];
497+
498+
assert!(matches!(hir_start, Expr::Path { .. }));
499+
assert!(matches!(hir_end, Expr::Path { .. }));
500+
}
501+
_ => {}
502+
}
503+
}

src/tools/rust-analyzer/crates/hir-def/src/hir.rs

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,12 +55,20 @@ impl ExprOrPatId {
5555
}
5656
}
5757

58+
pub fn is_expr(&self) -> bool {
59+
matches!(self, Self::ExprId(_))
60+
}
61+
5862
pub fn as_pat(self) -> Option<PatId> {
5963
match self {
6064
Self::PatId(v) => Some(v),
6165
_ => None,
6266
}
6367
}
68+
69+
pub fn is_pat(&self) -> bool {
70+
matches!(self, Self::PatId(_))
71+
}
6472
}
6573
stdx::impl_from!(ExprId, PatId for ExprOrPatId);
6674

@@ -571,8 +579,8 @@ pub enum Pat {
571579
ellipsis: bool,
572580
},
573581
Range {
574-
start: Option<Box<LiteralOrConst>>,
575-
end: Option<Box<LiteralOrConst>>,
582+
start: Option<ExprId>,
583+
end: Option<ExprId>,
576584
},
577585
Slice {
578586
prefix: Box<[PatId]>,

src/tools/rust-analyzer/crates/hir-ty/src/diagnostics/expr.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -440,7 +440,9 @@ impl ExprValidator {
440440
return;
441441
};
442442
let root = source_ptr.file_syntax(db.upcast());
443-
let ast::Expr::IfExpr(if_expr) = source_ptr.value.to_node(&root) else {
443+
let either::Left(ast::Expr::IfExpr(if_expr)) =
444+
source_ptr.value.to_node(&root)
445+
else {
444446
return;
445447
};
446448
let mut top_if_expr = if_expr;

src/tools/rust-analyzer/crates/hir-ty/src/mir/lower.rs

Lines changed: 9 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@ use hir_def::{
88
data::adt::{StructKind, VariantData},
99
expr_store::{Body, HygieneId},
1010
hir::{
11-
ArithOp, Array, BinaryOp, BindingAnnotation, BindingId, ExprId, LabelId, Literal,
12-
LiteralOrConst, MatchArm, Pat, PatId, RecordFieldPat, RecordLitField,
11+
ArithOp, Array, BinaryOp, BindingAnnotation, BindingId, ExprId, LabelId, Literal, MatchArm,
12+
Pat, PatId, RecordFieldPat, RecordLitField,
1313
},
1414
lang_item::{LangItem, LangItemTarget},
1515
path::Path,
@@ -1358,20 +1358,10 @@ impl<'ctx> MirLowerCtx<'ctx> {
13581358
Ok(())
13591359
}
13601360

1361-
fn lower_literal_or_const_to_operand(
1362-
&mut self,
1363-
ty: Ty,
1364-
loc: &LiteralOrConst,
1365-
) -> Result<Operand> {
1366-
match loc {
1367-
LiteralOrConst::Literal(l) => self.lower_literal_to_operand(ty, l),
1368-
LiteralOrConst::Const(c) => {
1369-
let c = match &self.body.pats[*c] {
1370-
Pat::Path(p) => p,
1371-
_ => not_supported!(
1372-
"only `char` and numeric types are allowed in range patterns"
1373-
),
1374-
};
1361+
fn lower_literal_or_const_to_operand(&mut self, ty: Ty, loc: &ExprId) -> Result<Operand> {
1362+
match &self.body.exprs[*loc] {
1363+
Expr::Literal(l) => self.lower_literal_to_operand(ty, l),
1364+
Expr::Path(c) => {
13751365
let edition = self.edition();
13761366
let unresolved_name =
13771367
|| MirLowerError::unresolved_path(self.db, c, edition, &self.body.types);
@@ -1392,6 +1382,9 @@ impl<'ctx> MirLowerCtx<'ctx> {
13921382
}
13931383
}
13941384
}
1385+
_ => {
1386+
not_supported!("only `char` and numeric types are allowed in range patterns");
1387+
}
13951388
}
13961389
}
13971390

src/tools/rust-analyzer/crates/hir-ty/src/mir/lower/pattern_matching.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
//! MIR lowering for patterns
22
3-
use hir_def::{hir::LiteralOrConst, AssocItemId};
3+
use hir_def::{hir::ExprId, AssocItemId};
44

55
use crate::{
66
mir::{
@@ -207,7 +207,7 @@ impl MirLowerCtx<'_> {
207207
)?
208208
}
209209
Pat::Range { start, end } => {
210-
let mut add_check = |l: &LiteralOrConst, binop| -> Result<()> {
210+
let mut add_check = |l: &ExprId, binop| -> Result<()> {
211211
let lv =
212212
self.lower_literal_or_const_to_operand(self.infer[pattern].clone(), l)?;
213213
let else_target = *current_else.get_or_insert_with(|| self.new_basic_block());

0 commit comments

Comments
 (0)