From 8280a1cbb3628beb8a179e7300724b3e84e58c46 Mon Sep 17 00:00:00 2001 From: Gavin Gray Date: Wed, 21 Feb 2024 21:36:06 +0100 Subject: [PATCH] Backup for debugging signals --- Cargo.toml | 2 +- crates/argus/src/analysis/transform.rs | 181 ++++++++---------- crates/argus/src/ext.rs | 24 +-- crates/argus/src/types.rs | 4 +- .../diesel/src/bad_insertable_field.rs | 2 +- 5 files changed, 100 insertions(+), 113 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index f367b46..61c8453 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [workspace] members = [ "crates/*" ] -exclude = [ "crates/argus_cli/tests/workspaces" ] +exclude = [ "crates/argus_cli/tests/workspaces", "examples" ] resolver = "2" [profile.dev.package.similar] diff --git a/crates/argus/src/analysis/transform.rs b/crates/argus/src/analysis/transform.rs index 0cc11b8..931a496 100644 --- a/crates/argus/src/analysis/transform.rs +++ b/crates/argus/src/analysis/transform.rs @@ -133,7 +133,7 @@ struct ObligationsBuilder<'a, 'tcx: 'a> { // Structures to be filled in exprs_to_hir_id: HashMap, ambiguity_errors: IndexSet, - trait_errors: IndexSet, + trait_errors: Vec<(ExprIdx, Vec)>, exprs: IndexVec, method_lookups: IndexVec, } @@ -151,7 +151,8 @@ impl<'a, 'tcx: 'a> ObligationsBuilder<'a, 'tcx> { kind, } in bins { - let span = hir.span_with_body(hir_id); + // let span = hir.span_with_body(hir_id); + let span = hir.span(hir_id); if let Some((range, snippet)) = CharRange::from_span(span, source_map).ok().and_then(|r| { let snip = source_map @@ -231,107 +232,91 @@ impl<'a, 'tcx: 'a> ObligationsBuilder<'a, 'tcx> { // FIXME: this isn't efficient, but the number of obligations per // body isn't large, so shouldnt' be an issue. fn relate_trait_bound(&mut self) { - // 1. take the expressions from the "reported_trait_errors" and find - // all the expressions that they correspond to. we should also - // maintain the order in which they are reported and use this - // sorting to present errors. for (span, predicates) in self.reported_trait_errors.iter() { - let Some(this_id) = - hier_hir::find_most_enclosing_node(&self.tcx, self.body_id, *span) - else { - log::error!("reported error doesn't have an associated span ..."); - continue; - }; - - let matching_expressions = self - .exprs_to_hir_id + // Search for the obligation hash in our set of computed obligations. + let predicates = predicates .iter() - .filter(|(_, that_id)| self.tcx.is_parent_of(**that_id, this_id)) + .filter_map(|&p| { + self + .raw_obligations + .iter_enumerated() + .find_map(|(obl_id, obl)| (obl.hash == p).then(|| (obl_id, p))) + }) .collect::>(); - let Some((expr_id, _hir_id)) = - matching_expressions.iter().copied().find(|(_, this_id)| { - matching_expressions - .iter() - .all(|(_, that_id)| self.tcx.is_parent_of(**that_id, **this_id)) - }) - else { - log::error!( - "failed to find most enclosing hir id for {:?}", - matching_expressions - ); - continue; - }; - - // Mark the found Expr as containing an error. - self.trait_errors.insert(*expr_id); - - // Sort the Expr obligations according to the reported order. - let expr_obligations = &mut self.exprs[*expr_id].obligations; - let num_errs = predicates.len(); - expr_obligations.sort_by_key(|&obl_idx| { - let obl = &self.raw_obligations[obl_idx]; - let obl_hash = obl.hash; - let obl_is_certain = obl.result.is_certain(); - predicates - .iter() - .position(|&h| h == obl_hash) - .unwrap_or_else(|| { - if obl_is_certain { - // push successful obligations down - num_errs + 1 - } else { - num_errs - } - }) - }) - } + // Associate these with an expression, first comes first served. + let mut root_expr = None; + 'outer: for (expr_id, expr) in self.exprs.iter_enumerated() { + for (p, _) in predicates.iter() { + if expr.obligations.contains(p) { + root_expr = Some(expr_id); + break 'outer; + } + } + } - // 2. we also need to search for expressions that are "ambiguous," these - // don't always have associated reported errors. my current thoughts to - // do this are to find obligations that are unsuccessful and have a - // concrete obligations code. - // - // TODO: this isn't quite doing what I want. We need a way to figure - // out which obligations are "reruns" of a previous goal, and - // then remove the prior 'ambiguous' answer from the list. - // - // let is_important_failed_query = |obl_idx: ObligationIdx| { - // use rustc_infer::traits::ObligationCauseCode::*; - // if let Some(prov) = self.obligations.iter().find(|p| ***p == obl_idx) - // && let Some(uodidx) = prov.full_data - // { - // let full_data = self.full_data.get(uodidx); - // !(full_data.result.is_certain() - // || matches!(full_data.obligation.cause.code(), MiscObligation)) - // } else { - // false - // } - // }; - // let lift_failed_obligations = |v: &mut Vec| { - // v.sort_by_key(|&idx| { - // if self.raw_obligations[idx].result.is_certain() { - // 1 - // } else { - // 0 - // } - // }) - // }; - // let unmarked_exprs = self - // .exprs - // .iter_mut_enumerated() - // .filter(|(id, _)| !self.ambiguity_errors.contains(id)); - // for (expr_id, expr) in unmarked_exprs { - // let contains_failed = expr - // .obligations - // .iter() - // .copied() - // .any(is_important_failed_query); - // if contains_failed { - // self.trait_errors.insert(expr_id); - // } - // lift_failed_obligations(&mut expr.obligations) - // } + let (_, hashes): (Vec, _) = predicates.into_iter().unzip(); + + if let Some(expr_id) = root_expr { + self.trait_errors.push((expr_id, hashes)); + } + // else { + // todo!("what should I do here?"); + // } + + // let Some(err_hir_id) = + // hier_hir::find_most_enclosing_node(&self.tcx, self.body_id, *span) + // else { + // log::error!("reported error doesn't have an associated span ..."); + // continue; + // }; + + // let parent_ids_of_error = self + // .exprs_to_hir_id + // .iter() + // .filter(|(_, expr_hir_id)| { + // self.tcx.is_parent_of(**expr_hir_id, err_hir_id) + // }) + // .collect::>(); + + // let Some((expr_id, _hir_id)) = + // parent_ids_of_error.iter().copied().find(|(_, this_id)| { + // // Find child-most expression that contains the error. + // parent_ids_of_error + // .iter() + // .all(|(_, that_id)| self.tcx.is_parent_of(**that_id, **this_id)) + // }) + // else { + // log::error!( + // "failed to find most enclosing hir id for {:?}", + // parent_ids_of_error + // ); + // continue; + // }; + + // // Mark the found Expr as containing an error. + // self.trait_errors.insert(*expr_id); + + // // Sort the Expr obligations according to the reported order. + // let expr_obligations = &mut self.exprs[*expr_id].obligations; + // let num_errs = predicates.len(); + // expr_obligations.sort_by_key(|&obl_idx| { + // let obl = &self.raw_obligations[obl_idx]; + // let obl_hash = obl.hash; + // let obl_is_certain = obl.result.is_certain(); + // predicates + // .iter() + // .position(|&h| h == obl_hash) + // .unwrap_or_else(|| { + // if obl_is_certain { + // // push successful obligations down + // num_errs + 1 + // } else { + // num_errs + // } + // }) + // }) + } } // 1. build the method call table (see ambiguous / ) diff --git a/crates/argus/src/ext.rs b/crates/argus/src/ext.rs index 5fb7144..65388f3 100644 --- a/crates/argus/src/ext.rs +++ b/crates/argus/src/ext.rs @@ -87,7 +87,7 @@ pub trait StableHash<'__ctx, 'tcx>: { fn stable_hash( self, - infcx: &InferCtxt<'tcx>, + infcx: &TyCtxt<'tcx>, ctx: &mut StableHashingContext<'__ctx>, ) -> Hash64; } @@ -107,6 +107,8 @@ pub trait TyCtxtExt<'tcx> { /// Test whether `a` is a parent node of `b`. fn is_parent_of(&self, a: HirId, b: HirId) -> bool; + + fn predicate_hash(&self, p: &Predicate<'tcx>) -> Hash64; } pub trait TypeckResultsExt<'tcx> { @@ -207,16 +209,12 @@ where { fn stable_hash( self, - infcx: &InferCtxt<'tcx>, + tcx: &TyCtxt<'tcx>, ctx: &mut StableHashingContext<'__ctx>, ) -> Hash64 { let mut h = StableHasher::new(); - let sans_regions = infcx.tcx.erase_regions(self); - let mut freshener = rustc_infer::infer::TypeFreshener::new(infcx); - // let mut eraser = ty_eraser::TyVarEraserVisitor { infcx }; - let this = sans_regions.fold_with(&mut freshener); - // erase infer vars - this.hash_stable(ctx, &mut h); + let sans_regions = tcx.erase_regions(self); + sans_regions.hash_stable(ctx, &mut h); h.finish() } } @@ -332,6 +330,10 @@ impl<'tcx> TyCtxtExt<'tcx> for TyCtxt<'tcx> { fn is_parent_of(&self, a: HirId, b: HirId) -> bool { a == b || self.hir().parent_iter(b).find(|&(id, _)| id == a).is_some() } + + fn predicate_hash(&self, p: &Predicate<'tcx>) -> Hash64 { + self.with_stable_hashing_context(|mut hcx| p.stable_hash(self, &mut hcx)) + } } impl<'tcx> TypeckResultsExt<'tcx> for TypeckResults<'tcx> { @@ -445,9 +447,9 @@ impl<'tcx> InferCtxtExt<'tcx> for InferCtxt<'tcx> { } fn predicate_hash(&self, p: &Predicate<'tcx>) -> Hash64 { - self - .tcx - .with_stable_hashing_context(|mut hcx| p.stable_hash(self, &mut hcx)) + let mut freshener = rustc_infer::infer::TypeFreshener::new(self); + let p = p.fold_with(&mut freshener); + self.tcx.predicate_hash(&p) } fn bless_fulfilled<'a>( diff --git a/crates/argus/src/types.rs b/crates/argus/src/types.rs index 91bc413..8c9d9f5 100644 --- a/crates/argus/src/types.rs +++ b/crates/argus/src/types.rs @@ -151,7 +151,7 @@ pub struct ObligationsInBody { /// Concrete trait errors, this would be when the compiler /// can say for certainty that a specific trait bound was required /// but not satisfied. - pub trait_errors: IndexSet, + pub trait_errors: Vec<(ExprIdx, Vec)>, #[cfg_attr(feature = "testing", ts(type = "Obligation[]"))] pub obligations: IndexVec, @@ -168,7 +168,7 @@ impl ObligationsInBody { id: Option<(&InferCtxt, DefId)>, range: CharRange, ambiguity_errors: IndexSet, - trait_errors: IndexSet, + trait_errors: Vec<(ExprIdx, Vec)>, obligations: IndexVec, exprs: IndexVec, method_lookups: IndexVec, diff --git a/crates/argus_cli/tests/workspaces/diesel/src/bad_insertable_field.rs b/crates/argus_cli/tests/workspaces/diesel/src/bad_insertable_field.rs index 9cc9c58..0518b90 100644 --- a/crates/argus_cli/tests/workspaces/diesel/src/bad_insertable_field.rs +++ b/crates/argus_cli/tests/workspaces/diesel/src/bad_insertable_field.rs @@ -9,5 +9,5 @@ table! { #[derive(Insertable)] struct User { - name: i32, + name: Text, }