Skip to content

Commit

Permalink
Actually use the #[do_not_recommend] attribute if present
Browse files Browse the repository at this point in the history
This change tweaks the error message generation to actually use the
`#[do_not_recommend]` attribute if present by just skipping the marked
trait impl in favour of the parent impl. It also adds a compile test for
this behaviour. Without this change the test would output the following
error:

```
error[E0277]: the trait bound `&str: Expression` is not satisfied
  --> /home/weiznich/Documents/rust/rust/tests/ui/diagnostic_namespace/do_not_recommend.rs:53:15
   |
LL |     SelectInt.check("bar");
   |               ^^^^^ the trait `Expression` is not implemented for `&str`, which is required by `&str: AsExpression<Integer>`
   |
   = help: the following other types implement trait `Expression`:
             Bound<T>
             SelectInt
note: required for `&str` to implement `AsExpression<Integer>`
  --> /home/weiznich/Documents/rust/rust/tests/ui/diagnostic_namespace/do_not_recommend.rs:26:13
   |
LL | impl<T, ST> AsExpression<ST> for T
   |             ^^^^^^^^^^^^^^^^     ^
LL | where
LL |     T: Expression<SqlType = ST>,
   |        ------------------------ unsatisfied trait bound introduced here
```

Note how that mentions `&str: Expression` before and now mentions `&str:
AsExpression<Integer>` instead which is much more helpful for users.

Open points for further changes before stabilization:

* We likely want to move the attribute to the `#[diagnostic]` namespace
to relax the guarantees given?
* How does it interact with the new trait solver?
  • Loading branch information
weiznich committed May 10, 2024
1 parent 80420a6 commit f0279ab
Show file tree
Hide file tree
Showing 3 changed files with 114 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -418,7 +418,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
match bound_predicate.skip_binder() {
ty::PredicateKind::Clause(ty::ClauseKind::Trait(trait_predicate)) => {
let trait_predicate = bound_predicate.rebind(trait_predicate);
let trait_predicate = self.resolve_vars_if_possible(trait_predicate);
let mut trait_predicate = self.resolve_vars_if_possible(trait_predicate);

// Let's use the root obligation as the main message, when we care about the
// most general case ("X doesn't implement Pattern<'_>") over the case that
Expand Down Expand Up @@ -464,6 +464,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
root_obligation,
)
} else {
let trait_predicate = self.apply_do_not_recommend(&mut trait_predicate, &mut obligation);
(trait_predicate, &obligation)
};
let trait_ref = main_trait_predicate.to_poly_trait_ref();
Expand Down Expand Up @@ -1000,6 +1001,51 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
err.emit()
}

fn apply_do_not_recommend<'b>(
&self,
trait_predicate: &'_ mut ty::Binder<'b, ty::TraitPredicate<'tcx>>,
obligation: &'_ mut PredicateObligation<'tcx>,
) -> ty::Binder<'b, ty::TraitPredicate<'tcx>>
where
'tcx: 'b,
{
let mut temp_trait_predicate = trait_predicate.clone();
let mut base_cause = obligation.cause.code().clone();
// we first need to check whether the current impl
// is marked as `#[do_not_recommend]`
if let ObligationCauseCode::ImplDerivedObligation(ref c) = base_cause {
if self.tcx.has_attr(c.impl_or_alias_def_id, sym::do_not_recommend) {
use std::ops::Deref;

let code = c.derived.parent_code.deref().clone();
obligation.cause.map_code(|_| code);
obligation.predicate = c.derived.parent_trait_pred.to_predicate(self.tcx);
temp_trait_predicate = c.derived.parent_trait_pred.clone();
*trait_predicate = c.derived.parent_trait_pred;
}
}

// later we need to walk the parent tree
// and check any impl there as well
while let Some((parent_cause, parent_pred)) = base_cause.parent() {
if let Some(parent_pred) = parent_pred {
if matches!(
parent_cause,
ObligationCauseCode::ImplDerivedObligation(ref c)
if self.tcx.has_attr(c.impl_or_alias_def_id, sym::do_not_recommend)
) {
let code = parent_cause.clone();
obligation.cause.map_code(|_| code);
obligation.predicate = parent_pred.to_predicate(self.tcx);
temp_trait_predicate = parent_pred.clone();
*trait_predicate = parent_pred;
}
}
base_cause = parent_cause.clone();
}
temp_trait_predicate
}

fn emit_specialized_closure_kind_error(
&self,
obligation: &PredicateObligation<'tcx>,
Expand Down
55 changes: 55 additions & 0 deletions tests/ui/diagnostic_namespace/do_not_recommend/as_expression.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
#![feature(do_not_recommend)]

pub trait Expression {
type SqlType;
}

pub trait AsExpression<ST> {
type Expression: Expression<SqlType = ST>;
}

pub struct Text;
pub struct Integer;

pub struct Bound<T>(T);
pub struct SelectInt;

impl Expression for SelectInt {
type SqlType = Integer;
}

impl<T> Expression for Bound<T> {
type SqlType = T;
}

#[do_not_recommend]
impl<T, ST> AsExpression<ST> for T
where
T: Expression<SqlType = ST>,
{
type Expression = T;
}

impl AsExpression<Integer> for i32 {
type Expression = Bound<Integer>;
}

impl AsExpression<Text> for &'_ str {
type Expression = Bound<Text>;
}

trait Foo: Expression + Sized {
fn check<T>(&self, _: T) -> <T as AsExpression<<Self as Expression>::SqlType>>::Expression
where
T: AsExpression<Self::SqlType>,
{
todo!()
}
}

impl<T> Foo for T where T: Expression {}

fn main() {
SelectInt.check("bar");
//~^ERROR the trait bound `&str: AsExpression<Integer>` is not satisfied
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
error[E0277]: the trait bound `&str: AsExpression<Integer>` is not satisfied
--> $DIR/as_expression.rs:53:15
|
LL | SelectInt.check("bar");
| ^^^^^ the trait `AsExpression<Integer>` is not implemented for `&str`
|
= help: the trait `AsExpression<Text>` is implemented for `&str`
= help: for that trait implementation, expected `Text`, found `Integer`

error: aborting due to 1 previous error

For more information about this error, try `rustc --explain E0277`.

0 comments on commit f0279ab

Please sign in to comment.