Skip to content

Commit eae47a4

Browse files
borsbors
committed
Auto merge of #54734 - pawroman:fix_range_borrowing_suggestion, r=varkor
Fix range literals borrowing suggestions Fixes #54505. The compiler issued incorrect range borrowing suggestions (missing `()` around borrows of range literals). This was not correct syntax (see the issue for an example). With changes in this PR, this is fixed for all types of `Range` literals. Thanks again to @varkor and @estebank for their invaluable help and guidance. r? @varkor
2 parents 96cafc5 + 1f7dafb commit eae47a4

9 files changed

+664
-3
lines changed

Diff for: ‎src/librustc_typeck/check/demand.rs

+72-3
Original file line numberDiff line numberDiff line change
@@ -309,11 +309,20 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
309309
};
310310
if self.can_coerce(ref_ty, expected) {
311311
if let Ok(src) = cm.span_to_snippet(sp) {
312-
let sugg_expr = match expr.node { // parenthesize if needed (Issue #46756)
312+
let needs_parens = match expr.node {
313+
// parenthesize if needed (Issue #46756)
313314
hir::ExprKind::Cast(_, _) |
314-
hir::ExprKind::Binary(_, _, _) => format!("({})", src),
315-
_ => src,
315+
hir::ExprKind::Binary(_, _, _) => true,
316+
// parenthesize borrows of range literals (Issue #54505)
317+
_ if self.is_range_literal(expr) => true,
318+
_ => false,
316319
};
320+
let sugg_expr = if needs_parens {
321+
format!("({})", src)
322+
} else {
323+
src
324+
};
325+
317326
if let Some(sugg) = self.can_use_as_ref(expr) {
318327
return Some(sugg);
319328
}
@@ -374,6 +383,66 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
374383
None
375384
}
376385

386+
/// This function checks if the specified expression is a built-in range literal.
387+
/// (See: `LoweringContext::lower_expr()` in `src/librustc/hir/lowering.rs`).
388+
fn is_range_literal(&self, expr: &hir::Expr) -> bool {
389+
use hir::{Path, QPath, ExprKind, TyKind};
390+
391+
// We support `::std::ops::Range` and `::core::ops::Range` prefixes
392+
let is_range_path = |path: &Path| {
393+
let mut segs = path.segments.iter()
394+
.map(|seg| seg.ident.as_str());
395+
396+
if let (Some(root), Some(std_core), Some(ops), Some(range), None) =
397+
(segs.next(), segs.next(), segs.next(), segs.next(), segs.next())
398+
{
399+
// "{{root}}" is the equivalent of `::` prefix in Path
400+
root == "{{root}}" && (std_core == "std" || std_core == "core")
401+
&& ops == "ops" && range.starts_with("Range")
402+
} else {
403+
false
404+
}
405+
};
406+
407+
let span_is_range_literal = |span: &Span| {
408+
// Check whether a span corresponding to a range expression
409+
// is a range literal, rather than an explicit struct or `new()` call.
410+
let source_map = self.tcx.sess.source_map();
411+
let end_point = source_map.end_point(*span);
412+
413+
if let Ok(end_string) = source_map.span_to_snippet(end_point) {
414+
!(end_string.ends_with("}") || end_string.ends_with(")"))
415+
} else {
416+
false
417+
}
418+
};
419+
420+
match expr.node {
421+
// All built-in range literals but `..=` and `..` desugar to Structs
422+
ExprKind::Struct(QPath::Resolved(None, ref path), _, _) |
423+
// `..` desugars to its struct path
424+
ExprKind::Path(QPath::Resolved(None, ref path)) => {
425+
return is_range_path(&path) && span_is_range_literal(&expr.span);
426+
}
427+
428+
// `..=` desugars into `::std::ops::RangeInclusive::new(...)`
429+
ExprKind::Call(ref func, _) => {
430+
if let ExprKind::Path(QPath::TypeRelative(ref ty, ref segment)) = func.node {
431+
if let TyKind::Path(QPath::Resolved(None, ref path)) = ty.node {
432+
let call_to_new = segment.ident.as_str() == "new";
433+
434+
return is_range_path(&path) && span_is_range_literal(&expr.span)
435+
&& call_to_new;
436+
}
437+
}
438+
}
439+
440+
_ => {}
441+
}
442+
443+
false
444+
}
445+
377446
pub fn check_for_cast(&self,
378447
err: &mut DiagnosticBuilder<'tcx>,
379448
expr: &hir::Expr,

Diff for: ‎src/test/ui/range/issue-54505-no-literals.fixed

+75
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
// run-rustfix
2+
3+
// Regression test for changes introduced while fixing #54505
4+
5+
// This test uses non-literals for Ranges
6+
// (expecting no parens with borrow suggestion)
7+
8+
use std::ops::RangeBounds;
9+
10+
11+
// take a reference to any built-in range
12+
fn take_range(_r: &impl RangeBounds<i8>) {}
13+
14+
15+
fn main() {
16+
take_range(&std::ops::Range { start: 0, end: 1 });
17+
//~^ ERROR mismatched types [E0308]
18+
//~| HELP consider borrowing here
19+
//~| SUGGESTION &std::ops::Range { start: 0, end: 1 }
20+
21+
take_range(&::std::ops::Range { start: 0, end: 1 });
22+
//~^ ERROR mismatched types [E0308]
23+
//~| HELP consider borrowing here
24+
//~| SUGGESTION &::std::ops::Range { start: 0, end: 1 }
25+
26+
take_range(&std::ops::RangeFrom { start: 1 });
27+
//~^ ERROR mismatched types [E0308]
28+
//~| HELP consider borrowing here
29+
//~| SUGGESTION &std::ops::RangeFrom { start: 1 }
30+
31+
take_range(&::std::ops::RangeFrom { start: 1 });
32+
//~^ ERROR mismatched types [E0308]
33+
//~| HELP consider borrowing here
34+
//~| SUGGESTION &::std::ops::RangeFrom { start: 1 }
35+
36+
take_range(&std::ops::RangeFull {});
37+
//~^ ERROR mismatched types [E0308]
38+
//~| HELP consider borrowing here
39+
//~| SUGGESTION &std::ops::RangeFull {}
40+
41+
take_range(&::std::ops::RangeFull {});
42+
//~^ ERROR mismatched types [E0308]
43+
//~| HELP consider borrowing here
44+
//~| SUGGESTION &::std::ops::RangeFull {}
45+
46+
take_range(&std::ops::RangeInclusive::new(0, 1));
47+
//~^ ERROR mismatched types [E0308]
48+
//~| HELP consider borrowing here
49+
//~| SUGGESTION &std::ops::RangeInclusive::new(0, 1)
50+
51+
take_range(&::std::ops::RangeInclusive::new(0, 1));
52+
//~^ ERROR mismatched types [E0308]
53+
//~| HELP consider borrowing here
54+
//~| SUGGESTION &::std::ops::RangeInclusive::new(0, 1)
55+
56+
take_range(&std::ops::RangeTo { end: 5 });
57+
//~^ ERROR mismatched types [E0308]
58+
//~| HELP consider borrowing here
59+
//~| SUGGESTION &std::ops::RangeTo { end: 5 }
60+
61+
take_range(&::std::ops::RangeTo { end: 5 });
62+
//~^ ERROR mismatched types [E0308]
63+
//~| HELP consider borrowing here
64+
//~| SUGGESTION &::std::ops::RangeTo { end: 5 }
65+
66+
take_range(&std::ops::RangeToInclusive { end: 5 });
67+
//~^ ERROR mismatched types [E0308]
68+
//~| HELP consider borrowing here
69+
//~| SUGGESTION &std::ops::RangeToInclusive { end: 5 }
70+
71+
take_range(&::std::ops::RangeToInclusive { end: 5 });
72+
//~^ ERROR mismatched types [E0308]
73+
//~| HELP consider borrowing here
74+
//~| SUGGESTION &::std::ops::RangeToInclusive { end: 5 }
75+
}

Diff for: ‎src/test/ui/range/issue-54505-no-literals.rs

+75
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
// run-rustfix
2+
3+
// Regression test for changes introduced while fixing #54505
4+
5+
// This test uses non-literals for Ranges
6+
// (expecting no parens with borrow suggestion)
7+
8+
use std::ops::RangeBounds;
9+
10+
11+
// take a reference to any built-in range
12+
fn take_range(_r: &impl RangeBounds<i8>) {}
13+
14+
15+
fn main() {
16+
take_range(std::ops::Range { start: 0, end: 1 });
17+
//~^ ERROR mismatched types [E0308]
18+
//~| HELP consider borrowing here
19+
//~| SUGGESTION &std::ops::Range { start: 0, end: 1 }
20+
21+
take_range(::std::ops::Range { start: 0, end: 1 });
22+
//~^ ERROR mismatched types [E0308]
23+
//~| HELP consider borrowing here
24+
//~| SUGGESTION &::std::ops::Range { start: 0, end: 1 }
25+
26+
take_range(std::ops::RangeFrom { start: 1 });
27+
//~^ ERROR mismatched types [E0308]
28+
//~| HELP consider borrowing here
29+
//~| SUGGESTION &std::ops::RangeFrom { start: 1 }
30+
31+
take_range(::std::ops::RangeFrom { start: 1 });
32+
//~^ ERROR mismatched types [E0308]
33+
//~| HELP consider borrowing here
34+
//~| SUGGESTION &::std::ops::RangeFrom { start: 1 }
35+
36+
take_range(std::ops::RangeFull {});
37+
//~^ ERROR mismatched types [E0308]
38+
//~| HELP consider borrowing here
39+
//~| SUGGESTION &std::ops::RangeFull {}
40+
41+
take_range(::std::ops::RangeFull {});
42+
//~^ ERROR mismatched types [E0308]
43+
//~| HELP consider borrowing here
44+
//~| SUGGESTION &::std::ops::RangeFull {}
45+
46+
take_range(std::ops::RangeInclusive::new(0, 1));
47+
//~^ ERROR mismatched types [E0308]
48+
//~| HELP consider borrowing here
49+
//~| SUGGESTION &std::ops::RangeInclusive::new(0, 1)
50+
51+
take_range(::std::ops::RangeInclusive::new(0, 1));
52+
//~^ ERROR mismatched types [E0308]
53+
//~| HELP consider borrowing here
54+
//~| SUGGESTION &::std::ops::RangeInclusive::new(0, 1)
55+
56+
take_range(std::ops::RangeTo { end: 5 });
57+
//~^ ERROR mismatched types [E0308]
58+
//~| HELP consider borrowing here
59+
//~| SUGGESTION &std::ops::RangeTo { end: 5 }
60+
61+
take_range(::std::ops::RangeTo { end: 5 });
62+
//~^ ERROR mismatched types [E0308]
63+
//~| HELP consider borrowing here
64+
//~| SUGGESTION &::std::ops::RangeTo { end: 5 }
65+
66+
take_range(std::ops::RangeToInclusive { end: 5 });
67+
//~^ ERROR mismatched types [E0308]
68+
//~| HELP consider borrowing here
69+
//~| SUGGESTION &std::ops::RangeToInclusive { end: 5 }
70+
71+
take_range(::std::ops::RangeToInclusive { end: 5 });
72+
//~^ ERROR mismatched types [E0308]
73+
//~| HELP consider borrowing here
74+
//~| SUGGESTION &::std::ops::RangeToInclusive { end: 5 }
75+
}

Diff for: ‎src/test/ui/range/issue-54505-no-literals.stderr

+147
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,147 @@
1+
error[E0308]: mismatched types
2+
--> $DIR/issue-54505-no-literals.rs:16:16
3+
|
4+
LL | take_range(std::ops::Range { start: 0, end: 1 });
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
6+
| |
7+
| expected reference, found struct `std::ops::Range`
8+
| help: consider borrowing here: `&std::ops::Range { start: 0, end: 1 }`
9+
|
10+
= note: expected type `&_`
11+
found type `std::ops::Range<{integer}>`
12+
13+
error[E0308]: mismatched types
14+
--> $DIR/issue-54505-no-literals.rs:21:16
15+
|
16+
LL | take_range(::std::ops::Range { start: 0, end: 1 });
17+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
18+
| |
19+
| expected reference, found struct `std::ops::Range`
20+
| help: consider borrowing here: `&::std::ops::Range { start: 0, end: 1 }`
21+
|
22+
= note: expected type `&_`
23+
found type `std::ops::Range<{integer}>`
24+
25+
error[E0308]: mismatched types
26+
--> $DIR/issue-54505-no-literals.rs:26:16
27+
|
28+
LL | take_range(std::ops::RangeFrom { start: 1 });
29+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
30+
| |
31+
| expected reference, found struct `std::ops::RangeFrom`
32+
| help: consider borrowing here: `&std::ops::RangeFrom { start: 1 }`
33+
|
34+
= note: expected type `&_`
35+
found type `std::ops::RangeFrom<{integer}>`
36+
37+
error[E0308]: mismatched types
38+
--> $DIR/issue-54505-no-literals.rs:31:16
39+
|
40+
LL | take_range(::std::ops::RangeFrom { start: 1 });
41+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
42+
| |
43+
| expected reference, found struct `std::ops::RangeFrom`
44+
| help: consider borrowing here: `&::std::ops::RangeFrom { start: 1 }`
45+
|
46+
= note: expected type `&_`
47+
found type `std::ops::RangeFrom<{integer}>`
48+
49+
error[E0308]: mismatched types
50+
--> $DIR/issue-54505-no-literals.rs:36:16
51+
|
52+
LL | take_range(std::ops::RangeFull {});
53+
| ^^^^^^^^^^^^^^^^^^^^^^
54+
| |
55+
| expected reference, found struct `std::ops::RangeFull`
56+
| help: consider borrowing here: `&std::ops::RangeFull {}`
57+
|
58+
= note: expected type `&_`
59+
found type `std::ops::RangeFull`
60+
61+
error[E0308]: mismatched types
62+
--> $DIR/issue-54505-no-literals.rs:41:16
63+
|
64+
LL | take_range(::std::ops::RangeFull {});
65+
| ^^^^^^^^^^^^^^^^^^^^^^^^
66+
| |
67+
| expected reference, found struct `std::ops::RangeFull`
68+
| help: consider borrowing here: `&::std::ops::RangeFull {}`
69+
|
70+
= note: expected type `&_`
71+
found type `std::ops::RangeFull`
72+
73+
error[E0308]: mismatched types
74+
--> $DIR/issue-54505-no-literals.rs:46:16
75+
|
76+
LL | take_range(std::ops::RangeInclusive::new(0, 1));
77+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
78+
| |
79+
| expected reference, found struct `std::ops::RangeInclusive`
80+
| help: consider borrowing here: `&std::ops::RangeInclusive::new(0, 1)`
81+
|
82+
= note: expected type `&_`
83+
found type `std::ops::RangeInclusive<{integer}>`
84+
85+
error[E0308]: mismatched types
86+
--> $DIR/issue-54505-no-literals.rs:51:16
87+
|
88+
LL | take_range(::std::ops::RangeInclusive::new(0, 1));
89+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
90+
| |
91+
| expected reference, found struct `std::ops::RangeInclusive`
92+
| help: consider borrowing here: `&::std::ops::RangeInclusive::new(0, 1)`
93+
|
94+
= note: expected type `&_`
95+
found type `std::ops::RangeInclusive<{integer}>`
96+
97+
error[E0308]: mismatched types
98+
--> $DIR/issue-54505-no-literals.rs:56:16
99+
|
100+
LL | take_range(std::ops::RangeTo { end: 5 });
101+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
102+
| |
103+
| expected reference, found struct `std::ops::RangeTo`
104+
| help: consider borrowing here: `&std::ops::RangeTo { end: 5 }`
105+
|
106+
= note: expected type `&_`
107+
found type `std::ops::RangeTo<{integer}>`
108+
109+
error[E0308]: mismatched types
110+
--> $DIR/issue-54505-no-literals.rs:61:16
111+
|
112+
LL | take_range(::std::ops::RangeTo { end: 5 });
113+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
114+
| |
115+
| expected reference, found struct `std::ops::RangeTo`
116+
| help: consider borrowing here: `&::std::ops::RangeTo { end: 5 }`
117+
|
118+
= note: expected type `&_`
119+
found type `std::ops::RangeTo<{integer}>`
120+
121+
error[E0308]: mismatched types
122+
--> $DIR/issue-54505-no-literals.rs:66:16
123+
|
124+
LL | take_range(std::ops::RangeToInclusive { end: 5 });
125+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
126+
| |
127+
| expected reference, found struct `std::ops::RangeToInclusive`
128+
| help: consider borrowing here: `&std::ops::RangeToInclusive { end: 5 }`
129+
|
130+
= note: expected type `&_`
131+
found type `std::ops::RangeToInclusive<{integer}>`
132+
133+
error[E0308]: mismatched types
134+
--> $DIR/issue-54505-no-literals.rs:71:16
135+
|
136+
LL | take_range(::std::ops::RangeToInclusive { end: 5 });
137+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
138+
| |
139+
| expected reference, found struct `std::ops::RangeToInclusive`
140+
| help: consider borrowing here: `&::std::ops::RangeToInclusive { end: 5 }`
141+
|
142+
= note: expected type `&_`
143+
found type `std::ops::RangeToInclusive<{integer}>`
144+
145+
error: aborting due to 12 previous errors
146+
147+
For more information about this error, try `rustc --explain E0308`.

0 commit comments

Comments
 (0)