Skip to content
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

some additional need_type_info.rs cleanup #97703

Merged
merged 5 commits into from
Jun 11, 2022
Merged
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
56 changes: 10 additions & 46 deletions compiler/rustc_infer/src/infer/error_reporting/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,8 @@ use rustc_hir::{Item, ItemKind, Node};
use rustc_middle::dep_graph::DepContext;
use rustc_middle::ty::print::with_no_trimmed_paths;
use rustc_middle::ty::{
self,
error::TypeError,
subst::{GenericArgKind, Subst, SubstsRef},
Binder, EarlyBinder, List, Region, Ty, TyCtxt, TypeFoldable, TypeSuperFoldable,
self, error::TypeError, Binder, List, Region, Subst, Ty, TyCtxt, TypeFoldable,
TypeSuperFoldable,
};
use rustc_span::{sym, symbol::kw, BytePos, DesugaringKind, Pos, Span};
use rustc_target::spec::abi;
Expand Down Expand Up @@ -926,10 +924,13 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
mut t1_out: &mut DiagnosticStyledString,
mut t2_out: &mut DiagnosticStyledString,
path: String,
sub: ty::subst::SubstsRef<'tcx>,
sub: &'tcx [ty::GenericArg<'tcx>],
other_path: String,
other_ty: Ty<'tcx>,
) -> Option<()> {
// FIXME/HACK: Go back to `SubstsRef` to use its inherent methods,
// ideally that shouldn't be necessary.
let sub = self.tcx.intern_substs(sub);
for (i, ta) in sub.types().enumerate() {
if ta == other_ty {
self.highlight_outer(&mut t1_out, &mut t2_out, path, sub, i, other_ty);
Expand Down Expand Up @@ -960,45 +961,6 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
}
}

/// For generic types with parameters with defaults, remove the parameters corresponding to
/// the defaults. This repeats a lot of the logic found in `ty::print::pretty`.
fn strip_generic_default_params(
&self,
def_id: DefId,
substs: ty::subst::SubstsRef<'tcx>,
) -> SubstsRef<'tcx> {
let generics = self.tcx.generics_of(def_id);
let mut num_supplied_defaults = 0;

let default_params = generics.params.iter().rev().filter_map(|param| match param.kind {
ty::GenericParamDefKind::Type { has_default: true, .. } => Some(param.def_id),
ty::GenericParamDefKind::Const { has_default: true } => Some(param.def_id),
_ => None,
});
for (def_id, actual) in iter::zip(default_params, substs.iter().rev()) {
match actual.unpack() {
GenericArgKind::Const(c) => {
if EarlyBinder(self.tcx.const_param_default(def_id)).subst(self.tcx, substs)
!= c
{
break;
}
}
GenericArgKind::Type(ty) => {
if self.tcx.bound_type_of(def_id).subst(self.tcx, substs) != ty {
break;
}
}
_ => break,
}
num_supplied_defaults += 1;
}
let len = generics.params.len();
let mut generics = generics.clone();
generics.params.truncate(len - num_supplied_defaults);
substs.truncate_to(self.tcx, &generics)
}

/// Given two `fn` signatures highlight only sub-parts that are different.
fn cmp_fn_sig(
&self,
Expand Down Expand Up @@ -1156,8 +1118,10 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
(&ty::Adt(def1, sub1), &ty::Adt(def2, sub2)) => {
let did1 = def1.did();
let did2 = def2.did();
let sub_no_defaults_1 = self.strip_generic_default_params(did1, sub1);
let sub_no_defaults_2 = self.strip_generic_default_params(did2, sub2);
let sub_no_defaults_1 =
self.tcx.generics_of(did1).own_substs_no_defaults(self.tcx, sub1);
let sub_no_defaults_2 =
self.tcx.generics_of(did2).own_substs_no_defaults(self.tcx, sub2);
let mut values = (DiagnosticStyledString::new(), DiagnosticStyledString::new());
let path1 = self.tcx.def_path_str(did1);
let path2 = self.tcx.def_path_str(did2);
Expand Down
48 changes: 21 additions & 27 deletions compiler/rustc_infer/src/infer/error_reporting/need_type_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use crate::infer::type_variable::TypeVariableOriginKind;
use crate::infer::InferCtxt;
use rustc_errors::{pluralize, struct_span_err, Applicability, DiagnosticBuilder, ErrorGuaranteed};
use rustc_hir as hir;
use rustc_hir::def::Res;
use rustc_hir::def::{CtorOf, DefKind, Namespace};
use rustc_hir::def_id::DefId;
use rustc_hir::intravisit::{self, Visitor};
Expand All @@ -11,7 +12,7 @@ use rustc_middle::infer::unify_key::ConstVariableOriginKind;
use rustc_middle::ty::adjustment::{Adjust, Adjustment, AutoBorrow, AutoBorrowMutability};
use rustc_middle::ty::print::{FmtPrinter, PrettyPrinter, Print, Printer};
use rustc_middle::ty::subst::{GenericArg, GenericArgKind, Subst, SubstsRef};
use rustc_middle::ty::{self, DefIdTree, GenericParamDefKind, InferConst};
use rustc_middle::ty::{self, DefIdTree, InferConst};
use rustc_middle::ty::{Ty, TyCtxt, TypeckResults};
use rustc_span::symbol::{kw, Ident};
use rustc_span::{BytePos, Span};
Expand Down Expand Up @@ -853,12 +854,23 @@ impl<'a, 'tcx> FindInferSourceVisitor<'a, 'tcx> {
hir::TyKind::Path(hir::QPath::Resolved(_self_ty, path)),
) => {
if tcx.res_generics_def_id(path.res) != Some(def.did()) {
bug!(
"unexpected path: def={:?} substs={:?} path={:?}",
def,
substs,
path,
);
match path.res {
Res::Def(DefKind::TyAlias, _) => {
// FIXME: Ideally we should support this. For that
// we have to map back from the self type to the
// type alias though. That's difficult.
//
// See the `need_type_info/type-alias.rs` test for
// some examples.
}
Comment on lines +858 to +865
Copy link
Contributor

Choose a reason for hiding this comment

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

@oli-obk you might want to chime in here

Copy link
Contributor

Choose a reason for hiding this comment

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

@GuillaumeGomez is working on it

Copy link
Member

Choose a reason for hiding this comment

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

Moving slowly but surely! (you can take a look at my work in progress there: https://github.com/rust-lang/rust/compare/master...GuillaumeGomez:rustc-middle-ty-alias?expand=1)

// There cannot be inference variables in the self type,
// so there's nothing for us to do here.
Res::SelfTy { .. } => {}
_ => warn!(
"unexpected path: def={:?} substs={:?} path={:?}",
def, substs, path,
),
Comment on lines +869 to +872
Copy link
Contributor

Choose a reason for hiding this comment

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

Is turning this into a warn instead of bug intended?

Copy link
Contributor Author

@lcnr lcnr Jun 3, 2022

Choose a reason for hiding this comment

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

yeah, don't want to ICE here and getting it wrong only worsens diagnostics.

feels safer than using a bug!

Copy link
Contributor

Choose a reason for hiding this comment

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

What about using delay_span_bug instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

delay_span_bug won't ever trigger, considering that we're always emitting an error

Copy link
Contributor

Choose a reason for hiding this comment

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

It does protect against a subsequent refactor making us hit this codepath without an error being emitted.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the FindInferSourceVisitor should not be relevant to whether we emit an error. I do not think this is an issue. Or at least I think the guard for that shouldn't be at this point. We currently guard against it by fn emit_inference_failure_err returning a DiagnosticBuilder<'tcx, ErrorGuaranteed>.

}
} else {
return Box::new(
self.resolved_path_inferred_subst_iter(path, substs)
Expand Down Expand Up @@ -958,26 +970,8 @@ impl<'a, 'tcx> Visitor<'tcx> for FindInferSourceVisitor<'a, 'tcx> {
generics.own_substs(substs).iter().position(|&arg| self.generic_arg_is_target(arg))
{
let substs = self.infcx.resolve_vars_if_possible(substs);
let num_args = generics
.params
.iter()
.rev()
.filter(|&p| !matches!(p.kind, GenericParamDefKind::Lifetime))
.skip_while(|&param| {
if let Some(default) = param.default_value(tcx) {
// FIXME: Using structural comparisions has a bunch of false negatives.
//
// We should instead try to replace inference variables with placeholders and
// then use `infcx.can_eq`. That probably should be a separate method
// generally used during error reporting.
default.subst(tcx, substs) == substs[param.index as usize]
} else {
false
}
})
.count();
let generic_args =
&generics.own_substs(substs)[generics.own_counts().lifetimes..][..num_args];
Copy link
Contributor Author

@lcnr lcnr Jun 8, 2022

Choose a reason for hiding this comment

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

that caused the ICE in #97698 (comment). Writing big PRs sure is difficult 😅

let generic_args = &generics.own_substs_no_defaults(tcx, substs)
[generics.own_counts().lifetimes..];
estebank marked this conversation as resolved.
Show resolved Hide resolved
let span = match expr.kind {
ExprKind::MethodCall(path, _, _) => path.ident.span,
_ => expr.span,
Expand Down
41 changes: 40 additions & 1 deletion compiler/rustc_middle/src/ty/generics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -228,8 +228,47 @@ impl<'tcx> Generics {
})
}

/// Returns the substs corresponding to the generic parameters
/// of this item, excluding `Self`.
///
/// **This should only be used for diagnostics purposes.**
pub fn own_substs_no_defaults(
&'tcx self,
tcx: TyCtxt<'tcx>,
substs: &'tcx [ty::GenericArg<'tcx>],
) -> &'tcx [ty::GenericArg<'tcx>] {
let mut own_params = self.parent_count..self.count();
if self.has_self && self.parent.is_none() {
own_params.start = 1;
}

// Filter the default arguments.
//
// This currently uses structural equality instead
// of semantic equivalance. While not ideal, that's
// good enough for now as this should only be used
// for diagnostics anyways.
own_params.end -= self
.params
.iter()
.rev()
.take_while(|param| {
param.default_value(tcx).map_or(false, |default| {
default.subst(tcx, substs) == substs[param.index as usize]
})
})
.count();

&substs[own_params]
}

/// Returns the substs corresponding to the generic parameters of this item, excluding `Self`.
pub fn own_substs(&'tcx self, substs: SubstsRef<'tcx>) -> &'tcx [ty::GenericArg<'tcx>] {
///
/// **This should only be used for diagnostics purposes.**
pub fn own_substs(
&'tcx self,
substs: &'tcx [ty::GenericArg<'tcx>],
) -> &'tcx [ty::GenericArg<'tcx>] {
let own = &substs[self.parent_count..][..self.params.len()];
if self.has_self && self.parent.is_none() { &own[1..] } else { &own }
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ use crate::mir::{Body, GeneratorLayout};
use crate::traits::{self, Reveal};
use crate::ty;
use crate::ty::fast_reject::SimplifiedType;
use crate::ty::subst::{GenericArg, InternalSubsts, Subst, SubstsRef};
use crate::ty::util::Discr;
pub use adt::*;
pub use assoc::*;
Expand All @@ -44,6 +43,7 @@ use rustc_session::cstore::CrateStoreDyn;
use rustc_span::symbol::{kw, sym, Ident, Symbol};
use rustc_span::Span;
use rustc_target::abi::Align;
pub use subst::*;
pub use vtable::*;

use std::fmt::Debug;
Expand Down
39 changes: 1 addition & 38 deletions compiler/rustc_middle/src/ty/print/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ pub trait Printer<'tcx>: Sized {
// on top of the same path, but without its own generics.
_ => {
if !generics.params.is_empty() && substs.len() >= generics.count() {
let args = self.generic_args_to_print(generics, substs);
let args = generics.own_substs_no_defaults(self.tcx(), substs);
return self.path_generic_args(
|cx| cx.print_def_path(def_id, parent_substs),
args,
Expand Down Expand Up @@ -184,43 +184,6 @@ pub trait Printer<'tcx>: Sized {
}
}

fn generic_args_to_print(
&self,
generics: &'tcx ty::Generics,
substs: &'tcx [GenericArg<'tcx>],
) -> &'tcx [GenericArg<'tcx>] {
let mut own_params = generics.parent_count..generics.count();

// Don't print args for `Self` parameters (of traits).
if generics.has_self && own_params.start == 0 {
own_params.start = 1;
}

// Don't print args that are the defaults of their respective parameters.
own_params.end -= generics
.params
.iter()
.rev()
.take_while(|param| match param.kind {
ty::GenericParamDefKind::Lifetime => false,
ty::GenericParamDefKind::Type { has_default, .. } => {
has_default
&& substs[param.index as usize]
== GenericArg::from(
self.tcx().bound_type_of(param.def_id).subst(self.tcx(), substs),
)
}
ty::GenericParamDefKind::Const { has_default } => {
has_default
&& substs[param.index as usize]
== GenericArg::from(self.tcx().const_param_default(param.def_id))
}
})
.count();

&substs[own_params]
}

fn default_print_impl_path(
self,
impl_def_id: DefId,
Expand Down
27 changes: 12 additions & 15 deletions compiler/rustc_middle/src/ty/print/pretty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -825,12 +825,11 @@ pub trait PrettyPrinter<'tcx>:

for (fn_once_trait_ref, entry) in fn_traits {
// Get the (single) generic ty (the args) of this FnOnce trait ref.
let generics = self.generic_args_to_print(
self.tcx().generics_of(fn_once_trait_ref.def_id()),
fn_once_trait_ref.skip_binder().substs,
);
let generics = self.tcx().generics_of(fn_once_trait_ref.def_id());
let args =
generics.own_substs_no_defaults(self.tcx(), fn_once_trait_ref.skip_binder().substs);

match (entry.return_ty, generics[0].expect_ty()) {
match (entry.return_ty, args[0].expect_ty()) {
// We can only print `impl Fn() -> ()` if we have a tuple of args and we recorded
// a return type.
(Some(return_ty), arg_tys) if matches!(arg_tys.kind(), ty::Tuple(_)) => {
Expand Down Expand Up @@ -892,15 +891,13 @@ pub trait PrettyPrinter<'tcx>:
print(trait_ref.skip_binder().print_only_trait_name())
);

let generics = self.generic_args_to_print(
self.tcx().generics_of(trait_ref.def_id()),
trait_ref.skip_binder().substs,
);
let generics = self.tcx().generics_of(trait_ref.def_id());
let args = generics.own_substs_no_defaults(self.tcx(), trait_ref.skip_binder().substs);

if !generics.is_empty() || !assoc_items.is_empty() {
if !args.is_empty() || !assoc_items.is_empty() {
let mut first = true;

for ty in generics {
for ty in args {
if first {
p!("<");
first = false;
Expand Down Expand Up @@ -1071,10 +1068,10 @@ pub trait PrettyPrinter<'tcx>:
let dummy_cx = cx.tcx().mk_ty_infer(ty::FreshTy(0));
let principal = principal.with_self_ty(cx.tcx(), dummy_cx);

let args = cx.generic_args_to_print(
cx.tcx().generics_of(principal.def_id),
principal.substs,
);
let args = cx
.tcx()
.generics_of(principal.def_id)
.own_substs_no_defaults(cx.tcx(), principal.substs);

// Don't print `'_` if there's no unerased regions.
let print_regions = args.iter().any(|arg| match arg.unpack() {
Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/const-generics/defaults/rp_impl_trait_fail.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,12 @@ impl Traitor<1, 2> for u64 {}


fn uwu<const N: u8>() -> impl Traitor<N> {
//~^ error: the trait bound `u32: Traitor<N, N>` is not satisfied
//~^ error: the trait bound `u32: Traitor<N>` is not satisfied
1_u32
}

fn owo() -> impl Traitor {
//~^ error: the trait bound `u64: Traitor<1_u8, 1_u8>` is not satisfied
//~^ error: the trait bound `u64: Traitor` is not satisfied
1_u64
}

Expand Down
8 changes: 4 additions & 4 deletions src/test/ui/const-generics/defaults/rp_impl_trait_fail.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -6,21 +6,21 @@ LL | fn rawr() -> impl Trait {
|
= help: the trait `Trait` is implemented for `Uwu<N>`

error[E0277]: the trait bound `u32: Traitor<N, N>` is not satisfied
error[E0277]: the trait bound `u32: Traitor<N>` is not satisfied
--> $DIR/rp_impl_trait_fail.rs:17:26
|
LL | fn uwu<const N: u8>() -> impl Traitor<N> {
| ^^^^^^^^^^^^^^^ the trait `Traitor<N, N>` is not implemented for `u32`
| ^^^^^^^^^^^^^^^ the trait `Traitor<N>` is not implemented for `u32`
lcnr marked this conversation as resolved.
Show resolved Hide resolved
|
= help: the following other types implement trait `Traitor<N, M>`:
<u32 as Traitor<N, 2_u8>>
<u64 as Traitor<1_u8, 2_u8>>

error[E0277]: the trait bound `u64: Traitor<1_u8, 1_u8>` is not satisfied
error[E0277]: the trait bound `u64: Traitor` is not satisfied
--> $DIR/rp_impl_trait_fail.rs:22:13
|
LL | fn owo() -> impl Traitor {
| ^^^^^^^^^^^^ the trait `Traitor<1_u8, 1_u8>` is not implemented for `u64`
| ^^^^^^^^^^^^ the trait `Traitor` is not implemented for `u64`
|
= help: the following other types implement trait `Traitor<N, M>`:
<u32 as Traitor<N, 2_u8>>
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/const-generics/defaults/trait_objects_fail.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,5 +26,5 @@ fn main() {
foo(&10_u32);
//~^ error: the trait bound `u32: Trait` is not satisfied
bar(&true);
//~^ error: the trait bound `bool: Traitor<{_: u8}, {_: u8}>` is not satisfied
//~^ error: the trait bound `bool: Traitor<{_: u8}>` is not satisfied
}
Loading