Skip to content

Commit 5e18788

Browse files
committed
Use HIR local analysis for resource retain escapes
1 parent acc4217 commit 5e18788

4 files changed

Lines changed: 66 additions & 55 deletions

File tree

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ It currently implements:
2525
- Per-function HIR body views that group bindings, calls, fields, effects, and returns
2626
- AST-driven mode and call checks for local-only features, named arguments, data effects, and retaining APIs
2727
- AST-driven body checks for local moves, early-exit-aware `fresh` returns, resource escape, resolved handle-field `take`, and managed closure captures
28-
- Local ownership effect application now indexes binding types and move/retain events from resolved HIR body trees
28+
- Local ownership and resource-retain escape checks now index binding types and move/retain events from resolved HIR body trees
2929
- Focused check modules for mode, calls, body semantics, and forbidden operator behavior
3030
- Fixture-based pass/fail scenario tests under `tests/fixtures`
3131

src/checks/body.rs

Lines changed: 39 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
use crate::analyzer::Analyzer;
22
use crate::diagnostic::{Diagnostic, code};
3-
use crate::hir::{
4-
CallResolution, HirEffectEventKind, HirReturnProof, HirTypeKind, ResolvedCalleeKind,
5-
};
3+
use crate::hir::{CallResolution, HirReturnProof, HirTypeKind, ResolvedCalleeKind};
64
use crate::syntax::ast::{Block, Callee, DataEffect, Expr, FunctionDecl, Item, LetKind, Stmt};
75

86
use super::local::{BodyState, LocalAnalysis, merge_if_state, merge_loop_state};
@@ -110,7 +108,7 @@ fn check_stmt_semantics(
110108
Flow::Return
111109
}
112110
Stmt::With(stmt) => {
113-
check_resource_escape(analyzer, &stmt.binding, &stmt.body);
111+
check_resource_escape(analyzer, local_analysis, &stmt.binding, &stmt.body);
114112
check_block(analyzer, local_analysis, function, &stmt.body, state)
115113
}
116114
Stmt::If(stmt) => {
@@ -383,7 +381,12 @@ fn check_take_of_handle_in_stmt(analyzer: &mut Analyzer<'_>, statement: &Stmt, s
383381
}
384382
}
385383

386-
fn check_resource_escape(analyzer: &mut Analyzer<'_>, binding: &str, body: &Block) {
384+
fn check_resource_escape(
385+
analyzer: &mut Analyzer<'_>,
386+
local_analysis: &LocalAnalysis,
387+
binding: &str,
388+
body: &Block,
389+
) {
387390
for statement in &body.statements {
388391
match statement {
389392
Stmt::Return(stmt) => {
@@ -393,7 +396,7 @@ fn check_resource_escape(analyzer: &mut Analyzer<'_>, binding: &str, body: &Bloc
393396
resource_escape_diagnostic(analyzer, binding, span.clone());
394397
}
395398
if let Some(value) = &stmt.value {
396-
check_resource_escape_expr(analyzer, binding, value);
399+
check_resource_escape_expr(analyzer, local_analysis, binding, value);
397400
}
398401
}
399402
Stmt::Let(stmt) => {
@@ -404,73 +407,73 @@ fn check_resource_escape(analyzer: &mut Analyzer<'_>, binding: &str, body: &Bloc
404407
resource_capture_diagnostic(analyzer, binding, span.clone());
405408
}
406409
if let Some(value) = &stmt.value {
407-
check_resource_escape_expr(analyzer, binding, value);
410+
check_resource_escape_expr(analyzer, local_analysis, binding, value);
408411
}
409412
}
410-
Stmt::Expr(expr) => check_resource_escape_expr(analyzer, binding, expr),
411-
Stmt::With(stmt) => check_resource_escape(analyzer, binding, &stmt.body),
413+
Stmt::Expr(expr) => check_resource_escape_expr(analyzer, local_analysis, binding, expr),
414+
Stmt::With(stmt) => {
415+
check_resource_escape(analyzer, local_analysis, binding, &stmt.body)
416+
}
412417
Stmt::If(stmt) => {
413-
check_resource_escape_expr(analyzer, binding, &stmt.condition);
414-
check_resource_escape(analyzer, binding, &stmt.then_body);
418+
check_resource_escape_expr(analyzer, local_analysis, binding, &stmt.condition);
419+
check_resource_escape(analyzer, local_analysis, binding, &stmt.then_body);
415420
if let Some(else_body) = &stmt.else_body {
416-
check_resource_escape(analyzer, binding, else_body);
421+
check_resource_escape(analyzer, local_analysis, binding, else_body);
417422
}
418423
}
419424
Stmt::Loop(stmt) => {
420425
if let Some(condition) = &stmt.condition {
421-
check_resource_escape_expr(analyzer, binding, condition);
426+
check_resource_escape_expr(analyzer, local_analysis, binding, condition);
422427
}
423-
check_resource_escape(analyzer, binding, &stmt.body);
428+
check_resource_escape(analyzer, local_analysis, binding, &stmt.body);
424429
}
425430
Stmt::Break(_) | Stmt::Continue(_) => {}
426431
Stmt::Unknown(_) => {}
427432
}
428433
}
429434
}
430435

431-
fn check_resource_escape_expr(analyzer: &mut Analyzer<'_>, binding: &str, expr: &Expr) {
436+
fn check_resource_escape_expr(
437+
analyzer: &mut Analyzer<'_>,
438+
local_analysis: &LocalAnalysis,
439+
binding: &str,
440+
expr: &Expr,
441+
) {
432442
match expr {
433443
Expr::Manage { value, span } => {
434444
if let Expr::Ident(name, _) = value.as_ref()
435445
&& name == binding
436446
{
437447
resource_escape_diagnostic(analyzer, binding, span.clone());
438448
}
439-
check_resource_escape_expr(analyzer, binding, value);
449+
check_resource_escape_expr(analyzer, local_analysis, binding, value);
450+
}
451+
Expr::Effect { value, .. } => {
452+
check_resource_escape_expr(analyzer, local_analysis, binding, value);
440453
}
441-
Expr::Effect { value, .. } => check_resource_escape_expr(analyzer, binding, value),
442454
Expr::Call { args, span, .. } => {
443-
check_resource_retained_by_call(analyzer, binding, span);
455+
check_resource_retained_by_call(analyzer, local_analysis, binding, span);
444456
for arg in args {
445-
check_resource_escape_expr(analyzer, binding, &arg.value);
457+
check_resource_escape_expr(analyzer, local_analysis, binding, &arg.value);
446458
}
447459
}
448-
Expr::Field { base, .. } => check_resource_escape_expr(analyzer, binding, base),
449-
Expr::Closure { body, .. } => check_resource_escape(analyzer, binding, body),
460+
Expr::Field { base, .. } => {
461+
check_resource_escape_expr(analyzer, local_analysis, binding, base);
462+
}
463+
Expr::Closure { body, .. } => {
464+
check_resource_escape(analyzer, local_analysis, binding, body)
465+
}
450466
Expr::Ident(_, _) | Expr::Number(_, _) | Expr::String(_, _) | Expr::Unknown(_) => {}
451467
}
452468
}
453469

454470
fn check_resource_retained_by_call(
455471
analyzer: &mut Analyzer<'_>,
472+
local_analysis: &LocalAnalysis,
456473
binding: &str,
457474
span: &crate::diagnostic::Span,
458475
) {
459-
let escaping_spans: Vec<crate::diagnostic::Span> = analyzer
460-
.hir
461-
.effect_events(span)
462-
.iter()
463-
.filter_map(|event| {
464-
if matches!(event.kind, HirEffectEventKind::Retain { .. })
465-
&& event.binding_name == binding
466-
{
467-
Some(event.value_span.clone())
468-
} else {
469-
None
470-
}
471-
})
472-
.collect();
473-
for escaping_span in escaping_spans {
476+
for escaping_span in local_analysis.retained_value_spans(span, binding) {
474477
resource_escape_diagnostic(analyzer, binding, escaping_span);
475478
}
476479
}

src/checks/local.rs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,21 @@ impl LocalAnalysis {
5858
state.apply_retention_events(self.effect_events(span));
5959
}
6060

61+
pub(crate) fn retained_value_spans(&self, span: &Span, binding: &str) -> Vec<Span> {
62+
self.effect_events(span)
63+
.iter()
64+
.filter_map(|event| {
65+
if matches!(event.kind, HirEffectEventKind::Retain { .. })
66+
&& event.binding_name == binding
67+
{
68+
Some(event.value_span.clone())
69+
} else {
70+
None
71+
}
72+
})
73+
.collect()
74+
}
75+
6176
pub(crate) fn binding_type(&self, span: &Span) -> Option<&str> {
6277
self.binding_types_by_span.get(span).map(String::as_str)
6378
}
@@ -528,6 +543,10 @@ mod tests {
528543

529544
assert_eq!(state.value_type("pool"), Some("ResourcePool<File>"));
530545
assert_eq!(local_analysis.binding_type(&span(2)), Some("Image"));
546+
assert_eq!(
547+
local_analysis.retained_value_spans(&span(20), "cached"),
548+
vec![span(20)]
549+
);
531550

532551
local_analysis.apply_retention_events(&span(20), &mut state);
533552
local_analysis.apply_move_events(&span(21), &mut state);

src/hir.rs

Lines changed: 7 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -283,7 +283,6 @@ pub struct Hir {
283283
field_accesses: Vec<HirFieldAccess>,
284284
field_accesses_by_span: HashMap<Span, HirFieldAccess>,
285285
effect_events: Vec<HirEffectEvent>,
286-
effect_events_by_span: HashMap<Span, Vec<HirEffectEvent>>,
287286
returns: Vec<HirReturn>,
288287
returns_by_span: HashMap<Span, HirReturn>,
289288
function_bodies: HashMap<String, HirFunctionBody>,
@@ -375,12 +374,6 @@ impl Hir {
375374
self.field_accesses_by_span.get(span)
376375
}
377376

378-
pub fn effect_events(&self, span: &Span) -> &[HirEffectEvent] {
379-
self.effect_events_by_span
380-
.get(span)
381-
.map_or(&[], Vec::as_slice)
382-
}
383-
384377
pub fn return_fact(&self, span: &Span) -> Option<&HirReturn> {
385378
self.returns_by_span.get(span)
386379
}
@@ -457,16 +450,6 @@ impl Hir {
457450
.iter()
458451
.map(|field| (field.span.clone(), field.clone()))
459452
.collect();
460-
self.effect_events_by_span = facts.effect_events.iter().fold(
461-
HashMap::<Span, Vec<HirEffectEvent>>::new(),
462-
|mut by_span, event| {
463-
by_span
464-
.entry(event.span.clone())
465-
.or_default()
466-
.push(event.clone());
467-
by_span
468-
},
469-
);
470453
self.returns_by_span = facts
471454
.returns
472455
.iter()
@@ -1946,7 +1929,13 @@ fn publish(cache: mut ImageCache, path: read Path) -> Unit {
19461929
HirEffectEventKind::Take
19471930
));
19481931
assert_eq!(hir.effect_events[2].binding_name, "image");
1949-
assert_eq!(hir.effect_events(&hir.effect_events[0].span).len(), 1);
1932+
assert_eq!(
1933+
hir.function_body("publish")
1934+
.expect("publish body exists")
1935+
.effect_events
1936+
.len(),
1937+
3
1938+
);
19501939
}
19511940

19521941
#[test]

0 commit comments

Comments
 (0)