Skip to content

Commit d70f5bc

Browse files
committed
forbid type ascriptions in lvalue contexts
1 parent 312c9e6 commit d70f5bc

File tree

7 files changed

+229
-23
lines changed

7 files changed

+229
-23
lines changed

compiler/rustc_middle/src/hir/map/mod.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -792,6 +792,13 @@ impl<'hir> Map<'hir> {
792792
}
793793
}
794794

795+
pub fn maybe_expr(&self, id: HirId) -> Option<&'hir Expr<'hir>> {
796+
match self.find(id) {
797+
Some(Node::Expr(expr)) => Some(expr),
798+
_ => None,
799+
}
800+
}
801+
795802
pub fn opt_name(&self, id: HirId) -> Option<Symbol> {
796803
Some(match self.get(id) {
797804
Node::Item(i) => i.ident.name,

compiler/rustc_middle/src/hir/place.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ use rustc_target::abi::VariantIdx;
1818
)]
1919
pub enum PlaceBase {
2020
/// A temporary variable.
21-
Rvalue,
21+
Rvalue(HirId),
2222
/// A named `static` item.
2323
StaticItem,
2424
/// A named local variable.

compiler/rustc_typeck/src/check/mod.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@ mod op;
8585
mod pat;
8686
mod place_op;
8787
mod regionck;
88+
mod type_ascription;
8889
mod upvar;
8990
mod wfcheck;
9091
pub mod writeback;
@@ -611,6 +612,8 @@ fn typeck_with_fallback<'tcx>(
611612
// backwards compatibility. This makes fallback a stronger type hint than a cast coercion.
612613
fcx.check_casts();
613614

615+
fcx.analyze_type_ascriptions(body);
616+
614617
// Closure and generator analysis may run after fallback
615618
// because they don't constrain other type variables.
616619
fcx.closure_analyze(body);

compiler/rustc_typeck/src/check/regionck.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -484,7 +484,7 @@ impl<'a, 'tcx> RegionCtxt<'a, 'tcx> {
484484
place_with_id: &PlaceWithHirId<'tcx>,
485485
span: Span,
486486
) {
487-
if let PlaceBase::Rvalue = place_with_id.place.base {
487+
if let PlaceBase::Rvalue(_) = place_with_id.place.base {
488488
if place_with_id.place.projections.is_empty() {
489489
let typ = self.resolve_type(place_with_id.place.ty());
490490
let body_id = self.body_id;
Lines changed: 186 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,186 @@
1+
use super::FnCtxt;
2+
use crate::expr_use_visitor::{ConsumeMode, Delegate, ExprUseVisitor};
3+
use rustc_errors::Applicability;
4+
use rustc_hir as hir;
5+
use rustc_hir::def::Res;
6+
use rustc_hir::intravisit::{self, Visitor};
7+
use rustc_middle::hir::place::{PlaceBase, PlaceWithHirId};
8+
use rustc_middle::ty;
9+
use rustc_span::hygiene::SyntaxContext;
10+
use rustc_span::symbol::Ident;
11+
use rustc_span::Span;
12+
13+
// Checks for type ascriptions in lvalue contexts, i.e. in situations such as
14+
// (x : T) = ..., &(x : T), &mut (x : T) and (x : T).foo(...), where
15+
// type ascriptions can potentially lead to type unsoundness problems
16+
struct TypeAscriptionValidator<'a, 'tcx> {
17+
fcx: &'a FnCtxt<'a, 'tcx>,
18+
finder: TypeAscriptionFinder<'a>,
19+
}
20+
21+
impl<'a, 'tcx> Delegate<'tcx> for TypeAscriptionValidator<'a, 'tcx> {
22+
fn consume(
23+
&mut self,
24+
_place_with_id: &PlaceWithHirId<'tcx>,
25+
_diag_expr_id: hir::HirId,
26+
_mode: ConsumeMode,
27+
) {
28+
}
29+
30+
fn borrow(
31+
&mut self,
32+
place_with_id: &PlaceWithHirId<'tcx>,
33+
_diag_expr_id: hir::HirId,
34+
bk: ty::BorrowKind,
35+
) {
36+
debug!(
37+
"TypeAscriptionkind::borrow(place: {:?}, borrow_kind: {:?})",
38+
place_with_id.place, bk
39+
);
40+
41+
if let PlaceBase::Rvalue(id) = place_with_id.place.base {
42+
if let Some(expr) = self.fcx.tcx.hir().maybe_expr(id) {
43+
debug!("expr behind place: {:?}", expr);
44+
self.finder.visit_expr(expr);
45+
if let Some(ascr_expr) = self.finder.found_type_ascr() {
46+
if let hir::ExprKind::Type(ref e, ref t) = ascr_expr.kind {
47+
let span = ascr_expr.span;
48+
let mut err = self.fcx.tcx.sess.struct_span_err(
49+
span,
50+
"type ascriptions are not allowed in lvalue contexts",
51+
);
52+
53+
if let Some((span, ident)) = self.maybe_get_span_ident_for_diagnostics(e) {
54+
if let Ok(ty_str) =
55+
self.fcx.tcx.sess.source_map().span_to_snippet(t.span)
56+
{
57+
err.span_suggestion(
58+
span,
59+
"try to use the type ascription when creating the local variable",
60+
format!("let {}: {}", ident, ty_str),
61+
Applicability::MaybeIncorrect,
62+
);
63+
}
64+
}
65+
err.emit();
66+
}
67+
}
68+
self.finder.reset();
69+
}
70+
}
71+
}
72+
73+
fn mutate(&mut self, assignee_place: &PlaceWithHirId<'tcx>, diag_expr_id: hir::HirId) {
74+
debug!(
75+
"TypeAscription::mutate(assignee_place: {:?}, diag_expr_id: {:?})",
76+
assignee_place, diag_expr_id
77+
);
78+
79+
if let PlaceBase::Rvalue(id) = assignee_place.place.base {
80+
if let Some(expr) = self.fcx.tcx.hir().maybe_expr(id) {
81+
debug!("expr behind place: {:?}", expr);
82+
if let hir::ExprKind::Type(_, _) = expr.kind {
83+
let span = expr.span;
84+
let mut err = self.fcx.tcx.sess.struct_span_err(
85+
span,
86+
"type ascriptions are not allowed in lvalue contexts",
87+
);
88+
err.emit();
89+
}
90+
}
91+
}
92+
}
93+
}
94+
95+
impl TypeAscriptionValidator<'a, 'tcx> {
96+
// Try to get the necessary information for an error suggestion, in case the
97+
// place on which the type ascription was used is a local variable.
98+
fn maybe_get_span_ident_for_diagnostics(&self, e: &hir::Expr<'_>) -> Option<(Span, Ident)> {
99+
match e.kind {
100+
hir::ExprKind::Path(hir::QPath::Resolved(_, path)) => {
101+
let hir::Path { res, segments, .. } = path;
102+
if let Res::Local(id) = res {
103+
// Span for the definition of the local variable
104+
let span = self.fcx.tcx.hir().span(*id);
105+
let source_map = self.fcx.tcx.sess.source_map();
106+
107+
if let Ok(file_lines) = source_map.span_to_lines(span) {
108+
let source_file = file_lines.file;
109+
let lines = file_lines.lines;
110+
111+
// Only create suggestion if the assignment operator is on the first line
112+
let line_bounds_range =
113+
lines.get(0).and_then(|l| Some(source_file.line_bounds(l.line_index)));
114+
if let Some(range) = line_bounds_range {
115+
let line_span =
116+
Span::new(range.start, range.end, SyntaxContext::root());
117+
if let Ok(line_string) = source_map.span_to_snippet(line_span) {
118+
if line_string.contains("=") {
119+
let span_til_eq = source_map.span_until_char(line_span, '=');
120+
if segments.len() == 1 {
121+
let ident = segments[0].ident;
122+
return Some((span_til_eq, ident));
123+
}
124+
}
125+
}
126+
}
127+
}
128+
}
129+
}
130+
_ => {}
131+
}
132+
None
133+
}
134+
}
135+
136+
impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
137+
pub fn analyze_type_ascriptions(&self, body: &'tcx hir::Body<'tcx>) {
138+
let body_owner_def_id = self.tcx.hir().body_owner_def_id(body.id());
139+
let finder = TypeAscriptionFinder::new();
140+
let mut delegate = TypeAscriptionValidator { fcx: self, finder };
141+
ExprUseVisitor::new(
142+
&mut delegate,
143+
&self.infcx,
144+
body_owner_def_id,
145+
self.param_env,
146+
&self.typeck_results.borrow(),
147+
)
148+
.consume_body(body);
149+
}
150+
}
151+
152+
struct TypeAscriptionFinder<'tcx> {
153+
found: Option<&'tcx hir::Expr<'tcx>>,
154+
}
155+
156+
impl<'tcx> TypeAscriptionFinder<'tcx> {
157+
fn new() -> TypeAscriptionFinder<'tcx> {
158+
TypeAscriptionFinder { found: None }
159+
}
160+
161+
fn found_type_ascr(&self) -> Option<&'tcx hir::Expr<'tcx>> {
162+
self.found
163+
}
164+
165+
fn reset(&mut self) {
166+
self.found = None;
167+
}
168+
}
169+
170+
impl Visitor<'tcx> for TypeAscriptionFinder<'tcx> {
171+
type Map = intravisit::ErasedMap<'tcx>;
172+
173+
fn nested_visit_map(&mut self) -> intravisit::NestedVisitorMap<Self::Map> {
174+
intravisit::NestedVisitorMap::None
175+
}
176+
177+
fn visit_expr(&mut self, expr: &'tcx hir::Expr<'tcx>) {
178+
match expr.kind {
179+
hir::ExprKind::Type(..) => {
180+
self.found = Some(expr);
181+
return;
182+
}
183+
_ => intravisit::walk_expr(self, expr),
184+
}
185+
}
186+
}

0 commit comments

Comments
 (0)