From 9c5cfea43da9b4e0372f240e01f58c977bd44d92 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Tue, 11 Aug 2015 10:48:34 -0400 Subject: [PATCH] traits: consider whether origin is RFC1214 when caching, ensuring that the test rfc1214-warn-and-error.rs reports an error --- src/librustc/middle/traits/fulfill.rs | 29 ++++++++++----- src/librustc/middle/traits/mod.rs | 9 +++++ src/librustc/middle/traits/select.rs | 4 +- .../compile-fail/rfc1214-warn-and-error.rs | 37 +++++++++++++++++++ 4 files changed, 68 insertions(+), 11 deletions(-) create mode 100644 src/test/compile-fail/rfc1214-warn-and-error.rs diff --git a/src/librustc/middle/traits/fulfill.rs b/src/librustc/middle/traits/fulfill.rs index 96637b92cef4..1fca66f13791 100644 --- a/src/librustc/middle/traits/fulfill.rs +++ b/src/librustc/middle/traits/fulfill.rs @@ -27,12 +27,13 @@ use super::ObligationCause; use super::ObligationCauseCode; use super::PredicateObligation; use super::project; +use super::RFC1214Warning; use super::select::SelectionContext; use super::Unimplemented; use super::util::predicate_for_builtin_bound; pub struct FulfilledPredicates<'tcx> { - set: HashSet> + set: HashSet<(RFC1214Warning, ty::Predicate<'tcx>)> } /// The fulfillment context is used to drive trait resolution. It @@ -190,7 +191,9 @@ impl<'tcx> FulfillmentContext<'tcx> { assert!(!obligation.has_escaping_regions()); - if self.is_duplicate_or_add(infcx.tcx, &obligation.predicate) { + let w = RFC1214Warning(obligation.cause.code.is_rfc1214()); + + if self.is_duplicate_or_add(infcx.tcx, w, &obligation.predicate) { debug!("register_predicate({:?}) -- already seen, skip", obligation); return; } @@ -253,7 +256,9 @@ impl<'tcx> FulfillmentContext<'tcx> { &self.predicates } - fn is_duplicate_or_add(&mut self, tcx: &ty::ctxt<'tcx>, + fn is_duplicate_or_add(&mut self, + tcx: &ty::ctxt<'tcx>, + w: RFC1214Warning, predicate: &ty::Predicate<'tcx>) -> bool { // This is a kind of dirty hack to allow us to avoid "rederiving" @@ -268,10 +273,12 @@ impl<'tcx> FulfillmentContext<'tcx> { // evaluating the 'nested obligations'. This cache lets us // skip those. - if self.errors_will_be_reported && predicate.is_global() { - tcx.fulfilled_predicates.borrow_mut().is_duplicate_or_add(predicate) + let will_warn_due_to_rfc1214 = w.0; + let errors_will_be_reported = self.errors_will_be_reported && !will_warn_due_to_rfc1214; + if errors_will_be_reported && predicate.is_global() { + tcx.fulfilled_predicates.borrow_mut().is_duplicate_or_add(w, predicate) } else { - self.duplicate_set.is_duplicate_or_add(predicate) + self.duplicate_set.is_duplicate_or_add(w, predicate) } } @@ -537,11 +544,13 @@ impl<'tcx> FulfilledPredicates<'tcx> { } } - pub fn is_duplicate(&self, p: &ty::Predicate<'tcx>) -> bool { - self.set.contains(p) + pub fn is_duplicate(&self, w: RFC1214Warning, p: &ty::Predicate<'tcx>) -> bool { + let key = (w, p.clone()); + self.set.contains(&key) } - fn is_duplicate_or_add(&mut self, p: &ty::Predicate<'tcx>) -> bool { - !self.set.insert(p.clone()) + fn is_duplicate_or_add(&mut self, w: RFC1214Warning, p: &ty::Predicate<'tcx>) -> bool { + let key = (w, p.clone()); + !self.set.insert(key) } } diff --git a/src/librustc/middle/traits/mod.rs b/src/librustc/middle/traits/mod.rs index 14ab6c505d05..6c501b1a609c 100644 --- a/src/librustc/middle/traits/mod.rs +++ b/src/librustc/middle/traits/mod.rs @@ -528,6 +528,15 @@ impl<'tcx> ObligationCause<'tcx> { } } +/// This marker is used in some caches to record whether the +/// predicate, if it is found to be false, will yield a warning (due +/// to RFC1214) or an error. We separate these two cases in the cache +/// so that if we see the same predicate twice, first resulting in a +/// warning, and next resulting in an error, we still report the +/// error, rather than considering it a duplicate. +#[derive(Copy, Clone, PartialEq, Eq, Hash)] +pub struct RFC1214Warning(bool); + impl<'tcx> ObligationCauseCode<'tcx> { pub fn is_rfc1214(&self) -> bool { match *self { diff --git a/src/librustc/middle/traits/select.rs b/src/librustc/middle/traits/select.rs index 713b7394b59a..f63523b77d60 100644 --- a/src/librustc/middle/traits/select.rs +++ b/src/librustc/middle/traits/select.rs @@ -27,6 +27,7 @@ use super::{ObligationCauseCode, BuiltinDerivedObligation, ImplDerivedObligation use super::{SelectionError, Unimplemented, OutputTypeParameterMismatch}; use super::{ObjectCastObligation, Obligation}; use super::TraitNotObjectSafe; +use super::RFC1214Warning; use super::Selection; use super::SelectionResult; use super::{VtableBuiltin, VtableImpl, VtableParam, VtableClosure, @@ -445,7 +446,8 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { // have been proven elsewhere. This cache only contains // predicates that are global in scope and hence unaffected by // the current environment. - if self.tcx().fulfilled_predicates.borrow().is_duplicate(&obligation.predicate) { + let w = RFC1214Warning(false); + if self.tcx().fulfilled_predicates.borrow().is_duplicate(w, &obligation.predicate) { return EvaluatedToOk; } diff --git a/src/test/compile-fail/rfc1214-warn-and-error.rs b/src/test/compile-fail/rfc1214-warn-and-error.rs new file mode 100644 index 000000000000..50fd3fc961c1 --- /dev/null +++ b/src/test/compile-fail/rfc1214-warn-and-error.rs @@ -0,0 +1,37 @@ +// Copyright 2015 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// Test that an RFC1214 warning from an earlier function (`foo`) does +// not suppress an error for the same problem (`WantEq`, +// `NotEq: !Eq`) in a later function (`bar)`. Earlier versions of the +// warning mechanism had an issue due to caching. + +#![allow(dead_code)] +#![allow(unused_variables)] + +struct WantEq { t: T } + +struct NotEq; + +trait Trait { } + +fn foo() { + let x: Box>> = loop { }; + //~^ WARN E0277 +} + +fn bar() { + wf::>(); + //~^ ERROR E0277 +} + +fn wf() { } + +fn main() { }