Skip to content

result_tracing: error return tracing in core #1

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: const-trait-specialize
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions compiler/rustc_span/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1132,6 +1132,7 @@ symbols! {
repr_transparent,
residual,
result,
result_tracing,
rhs,
rintf32,
rintf64,
Expand Down
70 changes: 46 additions & 24 deletions compiler/rustc_typeck/src/impl_wf_check/min_specialization.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ use rustc_middle::ty::trait_def::TraitSpecializationKind;
use rustc_middle::ty::{self, TyCtxt, TypeFoldable};
use rustc_span::Span;
use rustc_trait_selection::traits::{self, translate_substs, wf};
use tracing::instrument;

pub(super) fn check_min_specialization(tcx: TyCtxt<'_>, impl_def_id: DefId, span: Span) {
if let Some(node) = parent_specialization_node(tcx, impl_def_id) {
Expand All @@ -102,6 +103,7 @@ fn parent_specialization_node(tcx: TyCtxt<'_>, impl1_def_id: DefId) -> Option<No
}

/// Check that `impl1` is a sound specialization
#[instrument(level = "debug", skip(infcx))]
fn check_always_applicable(
infcx: &InferCtxt<'_, '_>,
impl1_def_id: DefId,
Expand All @@ -112,10 +114,8 @@ fn check_always_applicable(
get_impl_substs(infcx, impl1_def_id, impl2_node, span)
{
let impl2_def_id = impl2_node.def_id();
debug!(
"check_always_applicable(\nimpl1_def_id={:?},\nimpl2_def_id={:?},\nimpl2_substs={:?}\n)",
impl1_def_id, impl2_def_id, impl2_substs
);
debug!("impl2_def_id={impl2_def_id:?}");
debug!("impl2_substs={impl2_substs:?}");

let tcx = infcx.tcx;

Expand Down Expand Up @@ -278,10 +278,10 @@ fn check_static_lifetimes<'tcx>(
/// * global (not reference any parameters)
/// * `T: Tr` predicate where `Tr` is an always-applicable trait
/// * on the base `impl impl2`
/// * Currently this check is done using syntactic equality, which is
/// conservative but generally sufficient.
/// * This check is done using the `trait_predicates_eq` function below.
/// * a well-formed predicate of a type argument of the trait being implemented,
/// including the `Self`-type.
#[instrument(level = "debug", skip(infcx))]
fn check_predicates<'tcx>(
infcx: &InferCtxt<'_, 'tcx>,
impl1_def_id: LocalDefId,
Expand Down Expand Up @@ -313,10 +313,8 @@ fn check_predicates<'tcx>(
.map(|obligation| obligation.predicate)
.collect()
};
debug!(
"check_always_applicable(\nimpl1_predicates={:?},\nimpl2_predicates={:?}\n)",
impl1_predicates, impl2_predicates,
);
debug!("impl1_predicates={impl1_predicates:?}");
debug!("impl2_predicates={impl2_predicates:?}");

// Since impls of always applicable traits don't get to assume anything, we
// can also assume their supertraits apply.
Expand Down Expand Up @@ -362,25 +360,52 @@ fn check_predicates<'tcx>(
);

for predicate in impl1_predicates {
if !impl2_predicates.contains(&predicate) {
if !impl2_predicates.iter().any(|pred2| trait_predicates_eq(predicate, *pred2)) {
check_specialization_on(tcx, predicate, span)
}
}
}

/// Checks whether two predicates are the same for the purposes of specialization.
///
/// This is slightly more complicated than simple syntactic equivalence, since
/// we want to equate `T: Tr` with `T: ~const Tr` so this can work:
///
/// #[rustc_specialization_trait]
/// trait Specialize { }
///
/// impl<T: ~const Bound> const Tr for T { }
/// impl<T: Bound + Specialize> Tr for T { }
fn trait_predicates_eq<'tcx>(
predicate1: ty::Predicate<'tcx>,
predicate2: ty::Predicate<'tcx>,
) -> bool {
let predicate_kind_without_constness = |kind: ty::PredicateKind<'tcx>| match kind {
ty::PredicateKind::Trait(ty::TraitPredicate { trait_ref, constness: _, polarity }) => {
ty::PredicateKind::Trait(ty::TraitPredicate {
trait_ref,
constness: ty::BoundConstness::NotConst,
polarity,
})
}
_ => kind,
};

let pred1_kind_not_const = predicate1.kind().map_bound(predicate_kind_without_constness);
let pred2_kind_not_const = predicate2.kind().map_bound(predicate_kind_without_constness);

pred1_kind_not_const == pred2_kind_not_const
}

#[instrument(level = "debug", skip(tcx))]
fn check_specialization_on<'tcx>(tcx: TyCtxt<'tcx>, predicate: ty::Predicate<'tcx>, span: Span) {
debug!("can_specialize_on(predicate = {:?})", predicate);
match predicate.kind().skip_binder() {
// Global predicates are either always true or always false, so we
// are fine to specialize on.
_ if predicate.is_global() => (),
// We allow specializing on explicitly marked traits with no associated
// items.
ty::PredicateKind::Trait(ty::TraitPredicate {
trait_ref,
constness: ty::BoundConstness::NotConst,
polarity: _,
}) => {
ty::PredicateKind::Trait(ty::TraitPredicate { trait_ref, constness: _, polarity: _ }) => {
if !matches!(
trait_predicate_kind(tcx, predicate),
Some(TraitSpecializationKind::Marker)
Expand Down Expand Up @@ -409,13 +434,10 @@ fn trait_predicate_kind<'tcx>(
predicate: ty::Predicate<'tcx>,
) -> Option<TraitSpecializationKind> {
match predicate.kind().skip_binder() {
ty::PredicateKind::Trait(ty::TraitPredicate {
trait_ref,
constness: ty::BoundConstness::NotConst,
polarity: _,
}) => Some(tcx.trait_def(trait_ref.def_id).specialization_kind),
ty::PredicateKind::Trait(_)
| ty::PredicateKind::RegionOutlives(_)
ty::PredicateKind::Trait(ty::TraitPredicate { trait_ref, constness: _, polarity: _ }) => {
Some(tcx.trait_def(trait_ref.def_id).specialization_kind)
}
ty::PredicateKind::RegionOutlives(_)
| ty::PredicateKind::TypeOutlives(_)
| ty::PredicateKind::Projection(_)
| ty::PredicateKind::WellFormed(_)
Expand Down
66 changes: 64 additions & 2 deletions library/core/src/result.rs
Original file line number Diff line number Diff line change
Expand Up @@ -492,7 +492,7 @@
use crate::iter::{self, FromIterator, FusedIterator, TrustedLen};
use crate::marker::Destruct;
use crate::ops::{self, ControlFlow, Deref, DerefMut};
use crate::{convert, fmt, hint};
use crate::{convert, fmt, hint, panic};

/// `Result` is a type that represents either success ([`Ok`]) or failure ([`Err`]).
///
Expand Down Expand Up @@ -2101,14 +2101,76 @@ impl<T, E, F: ~const From<E>> const ops::FromResidual<Result<convert::Infallible
{
#[inline]
#[track_caller]
fn from_residual(residual: Result<convert::Infallible, E>) -> Self {
default fn from_residual(residual: Result<convert::Infallible, E>) -> Self {
match residual {
Err(e) => Err(From::from(e)),
}
}
}

// FIXME(bgr360): how to properly add this change that depends on compiler
// functionality that's not yet in the beta?
#[cfg(not(bootstrap))]
Comment on lines +2111 to +2113
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you did it right. The release team has a whole process for how they remove cfg(bootstrap) directives

https://forge.rust-lang.org/release/process.html?highlight=bootstrap#master-bootstrap-update-t-2-day-tuesday

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok dope

#[unstable(feature = "result_tracing", issue = "none")]
#[rustc_const_unstable(feature = "const_convert", issue = "88674")]
impl<T, E, F> const ops::FromResidual<Result<convert::Infallible, E>> for Result<T, F>
where
F: ~const From<E> + ~const Trace,
{
#[inline]
#[track_caller]
fn from_residual(residual: Result<convert::Infallible, E>) -> Self {
match residual {
Err(e) => {
let mut traced = F::from(e);
traced.trace(panic::Location::caller());
Err(traced)
}
}
}
}

#[unstable(feature = "try_trait_v2_residual", issue = "91285")]
impl<T, E> ops::Residual<T> for Result<convert::Infallible, E> {
type TryType = Result<T, E>;
}

/// A trait that enables try-tracing for [`Err`] variants of [`Result`].
///
/// Implementing this trait on your error type will notify you every time the
/// try operator (`?`) is invoked on your result, providing you with the source
/// code location of the `?` invocation.
///
/// # Example Usage
///
/// Given the following code:
///
/// ```
/// #![feature(result_tracing)]
/// #![feature(min_specialization)]
///
/// struct MyError;
///
/// impl std::result::Trace for MyError {
/// fn trace(&mut self, location: &'static std::panic::Location<'static>) {
/// println!("`?` invoked at {}", location);
/// }
/// }
///
/// # fn foo() -> Result<(), MyError> {
/// Err(MyError)?
/// # }
/// # foo().ok();
/// ```
///
/// Output like the following would be produced:
///
/// ```txt
/// `?` invoked at main.rs:LL:CC
/// ```
#[unstable(feature = "result_tracing", issue = "none")]
#[rustc_specialization_trait]
pub trait Trace {
/// Called during `?` with the source code location of the `?` invocation.
fn trace(&mut self, location: &'static panic::Location<'static>);
}
1 change: 1 addition & 0 deletions library/core/tests/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@
#![feature(never_type)]
#![feature(unwrap_infallible)]
#![feature(result_into_ok_or_err)]
#![feature(result_tracing)]
#![feature(portable_simd)]
#![feature(ptr_metadata)]
#![feature(once_cell)]
Expand Down
71 changes: 71 additions & 0 deletions library/core/tests/result.rs
Original file line number Diff line number Diff line change
Expand Up @@ -425,3 +425,74 @@ fn result_try_trait_v2_branch() {
assert_eq!(Ok::<NonZeroU32, ()>(one).branch(), Continue(one));
assert_eq!(Err::<NonZeroU32, ()>(()).branch(), Break(Err(())));
}

mod result_tracing {
use core::panic;
use core::result::Trace;

#[derive(Debug, PartialEq, Eq)]
struct TracedError(pub u32);

impl From<u32> for TracedError {
fn from(i: u32) -> Self {
Self(i)
}
}

impl Trace for TracedError {
// Increment by 1 on every `?` invocation.
fn trace(&mut self, _location: &'static panic::Location<'static>) {
self.0 += 1;
}
}

#[test]
fn try_operator_calls_trace() {
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: add const equivalent of this test

fn foo() -> Result<(), TracedError> {
Err(TracedError(0))
}

fn bar() -> Result<(), TracedError> {
Ok(foo()?)
}

fn baz() -> Result<(), TracedError> {
Ok(bar()?)
}

let result = baz();
assert_eq!(result, Err(TracedError(2)));
}

#[test]
fn try_operator_converts_into_traced_type_and_calls_trace() {
fn foo() -> Result<(), TracedError> {
Err(0)?
}

let result = foo();
assert_eq!(result, Err(TracedError(1)));
}

#[test]
fn try_operator_provides_correct_location() {
#[derive(Default)]
struct TheLocation(pub Option<&'static panic::Location<'static>>);

impl Trace for TheLocation {
fn trace(&mut self, location: &'static panic::Location<'static>) {
self.0 = Some(location);
}
}

let two_lines_before = panic::Location::caller();
fn foo() -> Result<(), TheLocation> {
Err(TheLocation::default())?
}

let result = foo();
let location = result.unwrap_err().0.unwrap();
assert_eq!(location.file(), two_lines_before.file());
assert_eq!(location.line(), two_lines_before.line() + 2);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
// Tests that a const default trait impl can be specialized by another const
// trait impl and that the specializing impl will be used during const-eval.

// run-pass

#![feature(const_trait_impl)]
#![feature(min_specialization)]

trait Value {
fn value() -> u32;
}

const fn get_value<T: ~const Value>() -> u32 {
T::value()
}

impl<T> const Value for T {
default fn value() -> u32 {
0
}
}

struct FortyTwo;

impl const Value for FortyTwo {
fn value() -> u32 {
42
}
}

const ZERO: u32 = get_value::<()>();

const FORTY_TWO: u32 = get_value::<FortyTwo>();

fn main() {
assert_eq!(ZERO, 0);
assert_eq!(FORTY_TWO, 42);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// Tests that a const default trait impl can be specialized by a non-const trait
// impl, but that the specializing impl cannot be used in a const context.

#![feature(const_trait_impl)]
#![feature(min_specialization)]

trait Value {
fn value() -> u32;
}

const fn get_value<T: ~const Value>() -> u32 {
// Ideally this error would show up at the call to `get_value`, not here.
T::value()
//~^ ERROR any use of this value will cause an error
//~| WARNING this was previously accepted
}

impl<T> const Value for T {
default fn value() -> u32 {
0
}
}

struct FortyTwo;

impl Value for FortyTwo {
fn value() -> u32 {
println!("You can't do that (constly)");
42
}
}

const ZERO: u32 = get_value::<()>();

const FORTY_TWO: u32 = get_value::<FortyTwo>();

fn main() {}
Loading