Skip to content

Make call suggestions more general and more accurate #101100

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

Merged
merged 7 commits into from
Aug 31, 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
19 changes: 15 additions & 4 deletions compiler/rustc_errors/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1248,9 +1248,13 @@ impl HandlerInner {
}

fn treat_err_as_bug(&self) -> bool {
self.flags
.treat_err_as_bug
.map_or(false, |c| self.err_count() + self.lint_err_count >= c.get())
self.flags.treat_err_as_bug.map_or(false, |c| {
Copy link
Member Author

@compiler-errors compiler-errors Aug 28, 2022

Choose a reason for hiding this comment

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

Well, I didn't mean to include this, but this is a fix to -Ztreat-err-as-bug which doesn't count delayed bugs correctly. I can remove it and put it into another PR if needed.

--

detailed explanation: If we emit an error A, delay a bug B, then emit error C. Then:

  1. running -Ztreat-err-as-bug=1 will make it ICE on A.
  2. running -Ztreat-err-as-bug=2 will make it ICE on B.
  3. running -Ztreat-err-as-bug=3 will skip over B and will make rust skip over C.

So there's literally no value N such that -Ztreat-err-as-bug=N will properly work to catch C. We add the delayed good path and span bugs into the count here so this works properly now.

self.err_count()
+ self.lint_err_count
+ self.delayed_span_bugs.len()
+ self.delayed_good_path_bugs.len()
>= c.get()
})
}

fn print_error_count(&mut self, registry: &Registry) {
Expand Down Expand Up @@ -1406,7 +1410,14 @@ impl HandlerInner {
// This is technically `self.treat_err_as_bug()` but `delay_span_bug` is called before
// incrementing `err_count` by one, so we need to +1 the comparing.
// FIXME: Would be nice to increment err_count in a more coherent way.
if self.flags.treat_err_as_bug.map_or(false, |c| self.err_count() + 1 >= c.get()) {
if self.flags.treat_err_as_bug.map_or(false, |c| {
self.err_count()
+ self.lint_err_count
+ self.delayed_span_bugs.len()
+ self.delayed_good_path_bugs.len()
+ 1
>= c.get()
}) {
// FIXME: don't abort here if report_delayed_bugs is off
self.span_bug(sp, msg);
}
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_lint_defs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@ macro_rules! pluralize {
/// All suggestions are marked with an `Applicability`. Tools use the applicability of a suggestion
/// to determine whether it should be automatically applied or if the user should be consulted
/// before applying the suggestion.
#[derive(Copy, Clone, Debug, PartialEq, Hash, Encodable, Decodable, Serialize, Deserialize)]
#[derive(Copy, Clone, Debug, Hash, Encodable, Decodable, Serialize, Deserialize)]
#[derive(PartialEq, Eq, PartialOrd, Ord)]
pub enum Applicability {
/// The suggestion is definitely what the user intended, or maintains the exact meaning of the code.
/// This suggestion should be automatically applied.
Expand Down
6 changes: 6 additions & 0 deletions compiler/rustc_middle/src/ty/structural_impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -844,6 +844,12 @@ impl<'tcx, T: TypeVisitable<'tcx>> TypeVisitable<'tcx> for Vec<T> {
}
}

impl<'tcx, T: TypeVisitable<'tcx>> TypeVisitable<'tcx> for &[T] {
fn visit_with<V: TypeVisitor<'tcx>>(&self, visitor: &mut V) -> ControlFlow<V::BreakTy> {
self.iter().try_for_each(|t| t.visit_with(visitor))
}
}

impl<'tcx, T: TypeFoldable<'tcx>> TypeFoldable<'tcx> for Box<[T]> {
fn try_fold_with<F: FallibleTypeFolder<'tcx>>(self, folder: &mut F) -> Result<Self, F::Error> {
self.try_map_id(|t| t.try_fold_with(folder))
Expand Down
102 changes: 42 additions & 60 deletions compiler/rustc_typeck/src/check/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ use crate::errors::{
};
use crate::type_error_struct;

use super::suggest_call_constructor;
use crate::errors::{AddressOfTemporaryTaken, ReturnStmtOutsideOfFnBody, StructExprNonExhaustive};
use rustc_ast as ast;
use rustc_data_structures::fx::FxHashMap;
Expand All @@ -44,7 +43,7 @@ use rustc_middle::middle::stability;
use rustc_middle::ty::adjustment::{Adjust, Adjustment, AllowTwoPhase};
use rustc_middle::ty::error::TypeError::FieldMisMatch;
use rustc_middle::ty::subst::SubstsRef;
use rustc_middle::ty::{self, AdtKind, DefIdTree, Ty, TypeVisitable};
use rustc_middle::ty::{self, AdtKind, Ty, TypeVisitable};
use rustc_session::parse::feature_err;
use rustc_span::hygiene::DesugaringKind;
use rustc_span::lev_distance::find_best_match_for_name;
Expand Down Expand Up @@ -2141,15 +2140,15 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
field: Ident,
) -> Ty<'tcx> {
debug!("check_field(expr: {:?}, base: {:?}, field: {:?})", expr, base, field);
let expr_t = self.check_expr(base);
let expr_t = self.structurally_resolved_type(base.span, expr_t);
let base_ty = self.check_expr(base);
let base_ty = self.structurally_resolved_type(base.span, base_ty);
let mut private_candidate = None;
let mut autoderef = self.autoderef(expr.span, expr_t);
while let Some((base_t, _)) = autoderef.next() {
debug!("base_t: {:?}", base_t);
match base_t.kind() {
let mut autoderef = self.autoderef(expr.span, base_ty);
while let Some((deref_base_ty, _)) = autoderef.next() {
debug!("deref_base_ty: {:?}", deref_base_ty);
match deref_base_ty.kind() {
ty::Adt(base_def, substs) if !base_def.is_enum() => {
debug!("struct named {:?}", base_t);
debug!("struct named {:?}", deref_base_ty);
let (ident, def_scope) =
self.tcx.adjust_ident_and_get_scope(field, base_def.did(), self.body_id);
let fields = &base_def.non_enum_variant().fields;
Expand Down Expand Up @@ -2197,23 +2196,23 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// (#90483) apply adjustments to avoid ExprUseVisitor from
// creating erroneous projection.
self.apply_adjustments(base, adjustments);
self.ban_private_field_access(expr, expr_t, field, did);
self.ban_private_field_access(expr, base_ty, field, did);
return field_ty;
}

if field.name == kw::Empty {
} else if self.method_exists(field, expr_t, expr.hir_id, true) {
self.ban_take_value_of_method(expr, expr_t, field);
} else if !expr_t.is_primitive_ty() {
self.ban_nonexisting_field(field, base, expr, expr_t);
} else if self.method_exists(field, base_ty, expr.hir_id, true) {
self.ban_take_value_of_method(expr, base_ty, field);
} else if !base_ty.is_primitive_ty() {
self.ban_nonexisting_field(field, base, expr, base_ty);
} else {
let field_name = field.to_string();
let mut err = type_error_struct!(
self.tcx().sess,
field.span,
expr_t,
base_ty,
E0610,
"`{expr_t}` is a primitive type and therefore doesn't have fields",
"`{base_ty}` is a primitive type and therefore doesn't have fields",
);
let is_valid_suffix = |field: &str| {
if field == "f32" || field == "f64" {
Expand Down Expand Up @@ -2251,7 +2250,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
None
}
};
if let ty::Infer(ty::IntVar(_)) = expr_t.kind()
if let ty::Infer(ty::IntVar(_)) = base_ty.kind()
&& let ExprKind::Lit(Spanned {
node: ast::LitKind::Int(_, ast::LitIntType::Unsuffixed),
..
Expand Down Expand Up @@ -2280,35 +2279,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
self.tcx().ty_error()
}

fn check_call_constructor(
&self,
err: &mut Diagnostic,
base: &'tcx hir::Expr<'tcx>,
def_id: DefId,
) {
if let Some(local_id) = def_id.as_local() {
let hir_id = self.tcx.hir().local_def_id_to_hir_id(local_id);
let node = self.tcx.hir().get(hir_id);

if let Some(fields) = node.tuple_fields() {
let kind = match self.tcx.opt_def_kind(local_id) {
Some(DefKind::Ctor(of, _)) => of,
_ => return,
};

suggest_call_constructor(base.span, kind, fields.len(), err);
}
} else {
// The logic here isn't smart but `associated_item_def_ids`
// doesn't work nicely on local.
if let DefKind::Ctor(of, _) = self.tcx.def_kind(def_id) {
let parent_def_id = self.tcx.parent(def_id);
let fields = self.tcx.associated_item_def_ids(parent_def_id);
suggest_call_constructor(base.span, of, fields.len(), err);
}
}
}

fn suggest_await_on_field_access(
&self,
err: &mut Diagnostic,
Expand Down Expand Up @@ -2351,40 +2321,52 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {

fn ban_nonexisting_field(
&self,
field: Ident,
ident: Ident,
base: &'tcx hir::Expr<'tcx>,
expr: &'tcx hir::Expr<'tcx>,
expr_t: Ty<'tcx>,
base_ty: Ty<'tcx>,
) {
debug!(
"ban_nonexisting_field: field={:?}, base={:?}, expr={:?}, expr_ty={:?}",
field, base, expr, expr_t
"ban_nonexisting_field: field={:?}, base={:?}, expr={:?}, base_ty={:?}",
ident, base, expr, base_ty
);
let mut err = self.no_such_field_err(field, expr_t, base.hir_id);
let mut err = self.no_such_field_err(ident, base_ty, base.hir_id);

match *expr_t.peel_refs().kind() {
match *base_ty.peel_refs().kind() {
ty::Array(_, len) => {
self.maybe_suggest_array_indexing(&mut err, expr, base, field, len);
self.maybe_suggest_array_indexing(&mut err, expr, base, ident, len);
}
ty::RawPtr(..) => {
self.suggest_first_deref_field(&mut err, expr, base, field);
self.suggest_first_deref_field(&mut err, expr, base, ident);
}
ty::Adt(def, _) if !def.is_enum() => {
self.suggest_fields_on_recordish(&mut err, def, field, expr.span);
self.suggest_fields_on_recordish(&mut err, def, ident, expr.span);
}
ty::Param(param_ty) => {
self.point_at_param_definition(&mut err, param_ty);
}
ty::Opaque(_, _) => {
self.suggest_await_on_field_access(&mut err, field, base, expr_t.peel_refs());
}
ty::FnDef(def_id, _) => {
self.check_call_constructor(&mut err, base, def_id);
self.suggest_await_on_field_access(&mut err, ident, base, base_ty.peel_refs());
}
_ => {}
}

if field.name == kw::Await {
self.suggest_fn_call(&mut err, base, base_ty, |output_ty| {
if let ty::Adt(def, _) = output_ty.kind() && !def.is_enum() {
def.non_enum_variant().fields.iter().any(|field| {
field.ident(self.tcx) == ident
&& field.vis.is_accessible_from(expr.hir_id.owner.to_def_id(), self.tcx)
})
} else if let ty::Tuple(tys) = output_ty.kind()
&& let Ok(idx) = ident.as_str().parse::<usize>()
{
idx < tys.len()
} else {
false
}
});

if ident.name == kw::Await {
// We know by construction that `<expr>.await` is either on Rust 2015
// or results in `ExprKind::Await`. Suggest switching the edition to 2018.
err.note("to `.await` a `Future`, switch to Rust 2018 or later");
Expand Down
Loading