diff --git a/crates/argus/src/analysis/entry.rs b/crates/argus/src/analysis/entry.rs index 94e9ebf..5fcb2b9 100644 --- a/crates/argus/src/analysis/entry.rs +++ b/crates/argus/src/analysis/entry.rs @@ -61,6 +61,19 @@ pub fn process_obligation<'tcx>( return; }; + // Use this to get rid of any resolved inference variables, + // these could have been resolved while trying to solve the obligation + // and we want to present it as such to the user. + let obl = &infcx.resolve_vars_if_possible(obl.clone()); + + // HACK: Remove ambiguous obligations if a "stronger" result was found and + // the predicate implies the previous. This is necessary because we + // can't (currently) distinguish between a subsequent solving attempt + // of a previous obligation. + if result.is_yes() || result.is_no() { + tls::drain_implied_ambiguities(infcx, &obl); + } + if !INCLUDE_SUCCESSES.copied().unwrap_or(false) && result.is_yes() { log::debug!("Skipping successful obligation {obl:?}"); return; @@ -68,11 +81,6 @@ pub fn process_obligation<'tcx>( log::debug!("Processing obligation {obl:?}"); - // Use this to get rid of any resolved inference variables, - // these could have been resolved while trying to solve the obligation - // and we want to present it as such to the user. - let obl = &infcx.resolve_vars_if_possible(obl.clone()); - // TODO: we need to figure out when to save the full data. // Saving it for every obligation consumes lots of memory // and this is (one of) the reasons the tool is relatively slow, diff --git a/crates/argus/src/analysis/tls.rs b/crates/argus/src/analysis/tls.rs index 517825d..f18162c 100644 --- a/crates/argus/src/analysis/tls.rs +++ b/crates/argus/src/analysis/tls.rs @@ -11,8 +11,9 @@ pub use unsafe_tls::{ }; use crate::{ - ext::InferCtxtExt, + ext::{EvaluationResultExt, InferCtxtExt}, proof_tree::SerializedTree, + rustc::InferCtxtExt as RustcInferCtxtExt, types::{intermediate::Provenance, Obligation, ObligationHash}, }; @@ -31,6 +32,70 @@ thread_local! { static REPORTED_ERRORS: RefCell>> = Default::default(); } +pub fn store_obligation(obl: Provenance) { + OBLIGATIONS.with(|obls| { + if obls + .borrow() + .iter() + .find(|o| *o.hash == *obl.hash) + .is_none() + { + obls.borrow_mut().push(obl) + } + }) +} + +pub fn drain_implied_ambiguities<'tcx>( + infcx: &InferCtxt<'tcx>, + obligation: &PredicateObligation<'tcx>, +) { + OBLIGATIONS.with(|obls| { + obls.borrow_mut().retain(|o| { + // Drain all elements that are: + // 1. Ambiguous, and-- + // 2. Implied by the passed obligation + let should_remove = o.result.is_maybe() + && o + .full_data + .map(|idx| { + unsafe_tls::borrow_in(idx, |fdata| { + fdata.infcx.universe() == infcx.universe() + && infcx.error_implies( + obligation.predicate.clone(), + fdata.obligation.predicate.clone(), + ) + }) + }) + .unwrap_or(false); + !should_remove + }) + }) +} + +pub fn take_obligations() -> Vec> { + OBLIGATIONS.with(|obls| obls.take()) +} + +pub fn replace_reported_errors(errs: FxIndexMap>) { + REPORTED_ERRORS.with(|rerrs| rerrs.replace(errs)); +} + +pub fn take_reported_errors() -> FxIndexMap> { + REPORTED_ERRORS.with(|rerrs| rerrs.take()) +} + +pub fn store_tree(new_tree: SerializedTree) { + TREE.with(|tree| { + if tree.borrow().is_none() { + tree.replace(Some(new_tree)); + } + }) +} + +pub fn take_tree() -> Option { + TREE.with(|tree| tree.take()) +} + // This is for complex obligations and their inference contexts. // We don't want to store the entire inference context and obligation for // every query, so we do it sparingly. @@ -88,6 +153,22 @@ mod unsafe_tls { }) } + // NOTE: ignore the 'tcx lifetime on the resulting reference. This data + // lives as long as the thread does, but the function can only be used + // from within this module so it shouldn't be an issue. + pub(super) fn borrow_in<'tcx, R>( + idx: UODIdx, + f: impl FnOnce(&'tcx FullObligationData<'tcx>) -> R, + ) -> R { + OBLIGATION_DATA.with(|data| { + let data = data.borrow(); + let ud = data.get(idx); + let d: &'tcx FullObligationData<'tcx> = + unsafe { std::mem::transmute(ud) }; + f(d) + }) + } + pub fn take<'tcx>() -> IndexVec> { OBLIGATION_DATA.with(|data| { data @@ -102,40 +183,3 @@ mod unsafe_tls { }) } } - -pub fn store_obligation(obl: Provenance) { - OBLIGATIONS.with(|obls| { - if obls - .borrow() - .iter() - .find(|o| *o.hash == *obl.hash) - .is_none() - { - obls.borrow_mut().push(obl) - } - }) -} - -pub fn take_obligations() -> Vec> { - OBLIGATIONS.with(|obls| obls.take()) -} - -pub fn replace_reported_errors(errs: FxIndexMap>) { - REPORTED_ERRORS.with(|rerrs| rerrs.replace(errs)); -} - -pub fn take_reported_errors() -> FxIndexMap> { - REPORTED_ERRORS.with(|rerrs| rerrs.take()) -} - -pub fn store_tree(new_tree: SerializedTree) { - TREE.with(|tree| { - if tree.borrow().is_none() { - tree.replace(Some(new_tree)); - } - }) -} - -pub fn take_tree() -> Option { - TREE.with(|tree| tree.take()) -} diff --git a/crates/argus/src/analysis/transform.rs b/crates/argus/src/analysis/transform.rs index d135193..069e318 100644 --- a/crates/argus/src/analysis/transform.rs +++ b/crates/argus/src/analysis/transform.rs @@ -1,5 +1,6 @@ use index_vec::IndexVec; use indexmap::IndexSet; +use itertools::Itertools; use rustc_data_structures::fx::{FxHashMap as HashMap, FxIndexMap}; use rustc_hir::{self as hir, intravisit::Map, BodyId, HirId}; use rustc_infer::{ @@ -398,6 +399,9 @@ impl<'a, 'tcx: 'a> ObligationsBuilder<'a, 'tcx> { }) .unzip(); + let trait_candidates = + trait_candidates.into_iter().unique().collect::>(); + let mut param_env = None; for &idx in &necessary_queries { let query = self.obligations[idx] diff --git a/crates/argus/src/ext.rs b/crates/argus/src/ext.rs index de38357..b996204 100644 --- a/crates/argus/src/ext.rs +++ b/crates/argus/src/ext.rs @@ -16,6 +16,7 @@ use rustc_middle::ty::{ TypeckResults, }; use rustc_query_system::ich::StableHashingContext; +use rustc_trait_selection::traits::solve::Certainty; use rustc_utils::source_map::range::CharRange; use serde::Serialize; @@ -116,6 +117,7 @@ pub trait TypeckResultsExt<'tcx> { pub trait EvaluationResultExt { fn is_yes(&self) -> bool; + fn is_maybe(&self) -> bool; fn is_no(&self) -> bool; } @@ -197,10 +199,13 @@ impl CharRangeExt for CharRange { impl EvaluationResultExt for EvaluationResult { fn is_yes(&self) -> bool { - use rustc_trait_selection::traits::solve::Certainty; matches!(self, EvaluationResult::Ok(Certainty::Yes)) } + fn is_maybe(&self) -> bool { + matches!(self, EvaluationResult::Ok(Certainty::Maybe(..))) + } + fn is_no(&self) -> bool { matches!(self, EvaluationResult::Err(..)) } diff --git a/crates/argus/src/rustc/mod.rs b/crates/argus/src/rustc/mod.rs index 1c169fb..7d558e0 100644 --- a/crates/argus/src/rustc/mod.rs +++ b/crates/argus/src/rustc/mod.rs @@ -7,13 +7,11 @@ //! The goal is that each copied block of code is modified minimally, //! making replacement easier in the future. - use rustc_infer::{ infer::InferCtxt, traits::{ - query::NoSolution, FulfillmentError, - FulfillmentErrorCode, MismatchedProjectionTypes, - PredicateObligation, SelectionError, + query::NoSolution, FulfillmentError, FulfillmentErrorCode, + MismatchedProjectionTypes, PredicateObligation, SelectionError, }, }; use rustc_middle::{ @@ -21,7 +19,6 @@ use rustc_middle::{ ty::error::{ExpectedFound, TypeError}, }; - use crate::types::intermediate::EvaluationResult; pub mod fn_ctx; @@ -38,6 +35,12 @@ pub trait InferCtxtExt<'tcx> { obligation: &PredicateObligation<'tcx>, result: EvaluationResult, ) -> Option>; + + fn error_implies( + &self, + cond: ty::Predicate<'tcx>, + error: ty::Predicate<'tcx>, + ) -> bool; } impl<'tcx> InferCtxtExt<'tcx> for InferCtxt<'tcx> { @@ -111,4 +114,56 @@ impl<'tcx> InferCtxtExt<'tcx> for InferCtxt<'tcx> { }, ) } + + fn error_implies( + &self, + cond: ty::Predicate<'tcx>, + error: ty::Predicate<'tcx>, + ) -> bool { + use rustc_middle::ty::ToPolyTraitRef; + use rustc_trait_selection::traits::elaborate; + + if cond == error { + return true; + } + + // FIXME: It should be possible to deal with `ForAll` in a cleaner way. + let bound_error = error.kind(); + let (cond, error) = + match (cond.kind().skip_binder(), bound_error.skip_binder()) { + ( + ty::PredicateKind::Clause(ty::ClauseKind::Trait(..)), + ty::PredicateKind::Clause(ty::ClauseKind::Trait(error)), + ) => (cond, bound_error.rebind(error)), + _ => { + // FIXME: make this work in other cases too. + return false; + } + }; + + for pred in elaborate(self.tcx, std::iter::once(cond)) { + let bound_predicate = pred.kind(); + if let ty::PredicateKind::Clause(ty::ClauseKind::Trait(implication)) = + bound_predicate.skip_binder() + { + let error = error.to_poly_trait_ref(); + let implication = bound_predicate.rebind(implication.trait_ref); + // FIXME: I'm just not taking associated types at all here. + // Eventually I'll need to implement param-env-aware + // `Γ₁ ⊦ φ₁ => Γ₂ ⊦ φ₂` logic. + let param_env = ty::ParamEnv::empty(); + if self.can_sub(param_env, error, implication) { + log::debug!( + "error_implies: {:?} -> {:?} -> {:?}", + cond, + error, + implication + ); + return true; + } + } + } + + false + } } diff --git a/ide/packages/panoptes/src/BodyInfo.ts b/ide/packages/panoptes/src/BodyInfo.ts index 4949cd3..e9535b1 100644 --- a/ide/packages/panoptes/src/BodyInfo.ts +++ b/ide/packages/panoptes/src/BodyInfo.ts @@ -4,6 +4,7 @@ import { ExprIdx, MethodLookup, MethodLookupIdx, + MethodStep, Obligation, ObligationHash, ObligationIdx, @@ -11,6 +12,8 @@ import { } from "@argus/common/bindings"; import _ from "lodash"; +import { isObject } from "./utilities/func"; + class BodyInfo { constructor( private readonly oib: ObligationsInBody, @@ -54,6 +57,26 @@ class BodyInfo { return this.oib.exprs[idx]; } + isErrorMethodCall(expr: Expr): boolean { + if (!(isObject(expr.kind) && "MethodCall" in expr.kind)) { + return false; + } + + if (expr.kind.MethodCall.errorRecvr) { + return true; + } + + const lookup = this.getMethodLookup(expr.kind.MethodCall.data); + + // This is an error method call if there doesn't exist an entry with a result "yes". + return !_.some(lookup.table, step => + _.some( + step.traitPredicates, + idx => this.getObligation(idx).result === "yes" + ) + ); + } + visibleObligations(idx: ExprIdx): ObligationIdx[] { const filtered = _.filter(this.oib.exprs[idx].obligations, i => this.notHidden(i) diff --git a/ide/packages/panoptes/src/File.tsx b/ide/packages/panoptes/src/File.tsx index ad712b9..492c93f 100644 --- a/ide/packages/panoptes/src/File.tsx +++ b/ide/packages/panoptes/src/File.tsx @@ -218,7 +218,7 @@ const InExpr = ({ idx }: { idx: ExprIdx }) => { ); if ( - !(isObject(expr.kind) && "MethodCall" in expr.kind) && + !bodyInfo.isErrorMethodCall(expr) && bodyInfo.visibleObligations(idx).length === 0 ) { return null; @@ -234,6 +234,7 @@ const InExpr = ({ idx }: { idx: ExprIdx }) => { ); // TODO: we should limit the length of the expression snippet. + // or at the very least syntax highlight it in some way... const header = 
{expr.snippet}
; const openChildren = idx === highlightedObligation.value?.exprIdx;