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

Don't do post-method-probe error reporting steps if we're in a suggestion #125100

Merged
merged 2 commits into from
May 14, 2024
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
4 changes: 2 additions & 2 deletions compiler/rustc_hir_typeck/src/demand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -938,14 +938,14 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
);
}

pub fn get_conversion_methods(
pub fn get_conversion_methods_for_diagnostic(
&self,
span: Span,
expected: Ty<'tcx>,
checked_ty: Ty<'tcx>,
hir_id: hir::HirId,
) -> Vec<AssocItem> {
let methods = self.probe_for_return_type(
let methods = self.probe_for_return_type_for_diagnostic(
span,
probe::Mode::MethodCall,
expected,
Expand Down
9 changes: 7 additions & 2 deletions compiler/rustc_hir_typeck/src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2414,7 +2414,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {

let guar = if field.name == kw::Empty {
self.dcx().span_delayed_bug(field.span, "field name with no name")
} else if self.method_exists(field, base_ty, expr.hir_id, expected.only_has_type(self)) {
} else if self.method_exists_for_diagnostic(
field,
base_ty,
expr.hir_id,
expected.only_has_type(self),
) {
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)
Expand Down Expand Up @@ -2600,7 +2605,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
let mut err = self.private_field_err(field, base_did);

// Also check if an accessible method exists, which is often what is meant.
if self.method_exists(field, expr_t, expr.hir_id, return_ty)
if self.method_exists_for_diagnostic(field, expr_t, expr.hir_id, return_ty)
&& !self.expr_in_place(expr.hir_id)
{
self.suggest_method_call(
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
expected_ty_expr: Option<&'tcx hir::Expr<'tcx>>,
) -> bool {
let expr = expr.peel_blocks();
let methods = self.get_conversion_methods(expr.span, expected, found, expr.hir_id);
let methods =
self.get_conversion_methods_for_diagnostic(expr.span, expected, found, expr.hir_id);

if let Some((suggestion, msg, applicability, verbose, annotation)) =
self.suggest_deref_or_ref(expr, found, expected)
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_hir_typeck/src/method/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ pub enum CandidateSource {
impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
/// Determines whether the type `self_ty` supports a visible method named `method_name` or not.
#[instrument(level = "debug", skip(self))]
pub fn method_exists(
pub fn method_exists_for_diagnostic(
Copy link
Contributor

Choose a reason for hiding this comment

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

I find the for_diagnostic suffix naming convention confusing. I think it means "method exists (as considered for diagnostic purposes)", rather than "method exists for this diagnostic"? It feels like there should be a contrasting method_exists_for_non_diagnostic method. I'm just not sure how to read it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, it means "and you should only call this method if you're doing diagnostics".

Copy link
Member Author

Choose a reason for hiding this comment

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

Kinda don't want to change it though now lol

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, I can live with it.

&self,
method_name: Ident,
self_ty: Ty<'tcx>,
Expand All @@ -102,7 +102,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
probe::Mode::MethodCall,
method_name,
return_type,
IsSuggestion(false),
IsSuggestion(true),
self_ty,
call_expr_id,
ProbeScope::TraitsInScope,
Expand Down
23 changes: 22 additions & 1 deletion compiler/rustc_hir_typeck/src/method/probe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,11 @@ pub(crate) struct ProbeContext<'a, 'tcx> {
>,

scope_expr_id: HirId,

/// Is this probe being done for a diagnostic? This will skip some error reporting
/// machinery, since we don't particularly care about, for example, similarly named
/// candidates if we're *reporting* similarly named candidates.
is_suggestion: IsSuggestion,
}

impl<'a, 'tcx> Deref for ProbeContext<'a, 'tcx> {
Expand Down Expand Up @@ -220,7 +225,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
/// would use to decide if a method is a plausible fit for
/// ambiguity purposes).
#[instrument(level = "debug", skip(self, candidate_filter))]
pub fn probe_for_return_type(
pub fn probe_for_return_type_for_diagnostic(
&self,
span: Span,
mode: Mode,
Expand Down Expand Up @@ -459,6 +464,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
&orig_values,
steps.steps,
scope_expr_id,
is_suggestion,
);

probe_cx.assemble_inherent_candidates();
Expand Down Expand Up @@ -553,6 +559,7 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
orig_steps_var_values: &'a OriginalQueryValues<'tcx>,
steps: &'tcx [CandidateStep<'tcx>],
scope_expr_id: HirId,
is_suggestion: IsSuggestion,
) -> ProbeContext<'a, 'tcx> {
ProbeContext {
fcx,
Expand All @@ -570,6 +577,7 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
static_candidates: RefCell::new(Vec::new()),
unsatisfied_predicates: RefCell::new(Vec::new()),
scope_expr_id,
is_suggestion,
}
}

Expand Down Expand Up @@ -944,6 +952,18 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
return r;
}

// If it's a `lookup_probe_for_diagnostic`, then quit early. No need to
// probe for other candidates.
if self.is_suggestion.0 {
return Err(MethodError::NoMatch(NoMatchData {
static_candidates: vec![],
unsatisfied_predicates: vec![],
out_of_scope_traits: vec![],
similar_candidate: None,
mode: self.mode,
}));
}

debug!("pick: actual search failed, assemble diagnostics");

let static_candidates = std::mem::take(self.static_candidates.get_mut());
Expand Down Expand Up @@ -1631,6 +1651,7 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
self.orig_steps_var_values,
self.steps,
self.scope_expr_id,
IsSuggestion(true),
);
pcx.allow_similar_names = true;
pcx.assemble_inherent_candidates();
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_hir_typeck/src/method/suggest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2814,7 +2814,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
Some(output_ty) => self.resolve_vars_if_possible(output_ty),
_ => return,
};
let method_exists = self.method_exists(item_name, output_ty, call.hir_id, return_type);
let method_exists =
self.method_exists_for_diagnostic(item_name, output_ty, call.hir_id, return_type);
debug!("suggest_await_before_method: is_method_exist={}", method_exists);
if method_exists {
err.span_suggestion_verbose(
Expand Down
163 changes: 163 additions & 0 deletions tests/ui/methods/probe-for-diagnostic-doesnt-do-extra-work.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,163 @@
#![allow(non_snake_case)]
Copy link
Contributor

Choose a reason for hiding this comment

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

Excellent that you have a test case! Please add a brief comment explaining what this is testing and pointing to this PR.


// Ensures that we don't spend an unnecessary amount of compiler time building
// suggestions for mistyped methods. This used to happen because when we had method
// lookup failures, we would do subsequent method lookups, and those would themselves
// do extra work that was unnecessary for the purposes of suggestions.
// Fixed by #125100.

macro_rules! traits {
($($name:ident),*) => {
$(
trait $name {
fn $name(&self);
}
)*
}
}

traits!(
A0, A1, A2, A3, A4, A5, A6, A7, A8, A9, A10, A11, A12, A13, A14, A15, A16, A17, A18, A19, A20,
A21, A22, A23, A24, A25, A26, A27, A28, A29, A30, A31, A32, A33, A34, A35, A36, A37, A38, A39,
A40, A41, A42, A43, A44, A45, A46, A47, A48, A49, A50, A51, A52, A53, A54, A55, A56, A57, A58,
A59, A60, A61, A62, A63, A64, A65, A66, A67, A68, A69, A70, A71, A72, A73, A74, A75, A76, A77,
A78, A79, A80, A81, A82, A83, A84, A85, A86, A87, A88, A89, A90, A91, A92, A93, A94, A95, A96,
A97, A98, A99, A100, A101, A102, A103, A104, A105, A106, A107, A108, A109, A110, A111, A112,
A113, A114, A115, A116, A117, A118, A119, A120, A121, A122, A123, A124, A125, A126, A127, A128,
A129, A130, A131, A132, A133, A134, A135, A136, A137, A138, A139, A140, A141, A142, A143, A144,
A145, A146, A147, A148, A149, A150, A151, A152, A153, A154, A155, A156, A157, A158, A159, A160,
A161, A162, A163, A164, A165, A166, A167, A168, A169, A170, A171, A172, A173, A174, A175, A176,
A177, A178, A179, A180, A181, A182, A183, A184, A185, A186, A187, A188, A189, A190, A191, A192,
A193, A194, A195, A196, A197, A198, A199, A200, A201, A202, A203, A204, A205, A206, A207, A208,
A209, A210, A211, A212, A213, A214, A215, A216, A217, A218, A219, A220, A221, A222, A223, A224,
A225, A226, A227, A228, A229, A230, A231, A232, A233, A234, A235, A236, A237, A238, A239, A240,
A241, A242, A243, A244, A245, A246, A247, A248, A249, A250, A251, A252, A253, A254, A255, A256,
A257, A258, A259, A260, A261, A262, A263, A264, A265, A266, A267, A268, A269, A270, A271, A272,
A273, A274, A275, A276, A277, A278, A279, A280, A281, A282, A283, A284, A285, A286, A287, A288,
A289, A290, A291, A292, A293, A294, A295, A296, A297, A298, A299, A300, A301, A302, A303, A304,
A305, A306, A307, A308, A309, A310, A311, A312, A313, A314, A315, A316, A317, A318, A319, A320,
A321, A322, A323, A324, A325, A326, A327, A328, A329, A330, A331, A332, A333, A334, A335, A336,
A337, A338, A339, A340, A341, A342, A343, A344, A345, A346, A347, A348, A349, A350, A351, A352,
A353, A354, A355, A356, A357, A358, A359, A360, A361, A362, A363, A364, A365, A366, A367, A368,
A369, A370, A371, A372, A373, A374, A375, A376, A377, A378, A379, A380, A381, A382, A383, A384,
A385, A386, A387, A388, A389, A390, A391, A392, A393, A394, A395, A396, A397, A398, A399, A400,
A401, A402, A403, A404, A405, A406, A407, A408, A409, A410, A411, A412, A413, A414, A415, A416,
A417, A418, A419, A420, A421, A422, A423, A424, A425, A426, A427, A428, A429, A430, A431, A432,
A433, A434, A435, A436, A437, A438, A439, A440, A441, A442, A443, A444, A445, A446, A447, A448,
A449, A450, A451, A452, A453, A454, A455, A456, A457, A458, A459, A460, A461, A462, A463, A464,
A465, A466, A467, A468, A469, A470, A471, A472, A473, A474, A475, A476, A477, A478, A479, A480,
A481, A482, A483, A484, A485, A486, A487, A488, A489, A490, A491, A492, A493, A494, A495, A496,
A497, A498, A499, A500, A501, A502, A503, A504, A505, A506, A507, A508, A509, A510, A511, A512,
A513, A514, A515, A516, A517, A518, A519, A520, A521, A522, A523, A524, A525, A526, A527, A528,
A529, A530, A531, A532, A533, A534, A535, A536, A537, A538, A539, A540, A541, A542, A543, A544,
A545, A546, A547, A548, A549, A550, A551, A552, A553, A554, A555, A556, A557, A558, A559, A560,
A561, A562, A563, A564, A565, A566, A567, A568, A569, A570, A571, A572, A573, A574, A575, A576,
A577, A578, A579, A580, A581, A582, A583, A584, A585, A586, A587, A588, A589, A590, A591, A592,
A593, A594, A595, A596, A597, A598, A599, A600, A601, A602, A603, A604, A605, A606, A607, A608,
A609, A610, A611, A612, A613, A614, A615, A616, A617, A618, A619, A620, A621, A622, A623, A624,
A625, A626, A627, A628, A629, A630, A631, A632, A633, A634, A635, A636, A637, A638, A639, A640,
A641, A642, A643, A644, A645, A646, A647, A648, A649, A650, A651, A652, A653, A654, A655, A656,
A657, A658, A659, A660, A661, A662, A663, A664, A665, A666, A667, A668, A669, A670, A671, A672,
A673, A674, A675, A676, A677, A678, A679, A680, A681, A682, A683, A684, A685, A686, A687, A688,
A689, A690, A691, A692, A693, A694, A695, A696, A697, A698, A699, A700, A701, A702, A703, A704,
A705, A706, A707, A708, A709, A710, A711, A712, A713, A714, A715, A716, A717, A718, A719, A720,
A721, A722, A723, A724, A725, A726, A727, A728, A729, A730, A731, A732, A733, A734, A735, A736,
A737, A738, A739, A740, A741, A742, A743, A744, A745, A746, A747, A748, A749, A750, A751, A752,
A753, A754, A755, A756, A757, A758, A759, A760, A761, A762, A763, A764, A765, A766, A767, A768,
A769, A770, A771, A772, A773, A774, A775, A776, A777, A778, A779, A780, A781, A782, A783, A784,
A785, A786, A787, A788, A789, A790, A791, A792, A793, A794, A795, A796, A797, A798, A799, A800,
A801, A802, A803, A804, A805, A806, A807, A808, A809, A810, A811, A812, A813, A814, A815, A816,
A817, A818, A819, A820, A821, A822, A823, A824, A825, A826, A827, A828, A829, A830, A831, A832,
A833, A834, A835, A836, A837, A838, A839, A840, A841, A842, A843, A844, A845, A846, A847, A848,
A849, A850, A851, A852, A853, A854, A855, A856, A857, A858, A859, A860, A861, A862, A863, A864,
A865, A866, A867, A868, A869, A870, A871, A872, A873, A874, A875, A876, A877, A878, A879, A880,
A881, A882, A883, A884, A885, A886, A887, A888, A889, A890, A891, A892, A893, A894, A895, A896,
A897, A898, A899, A900, A901, A902, A903, A904, A905, A906, A907, A908, A909, A910, A911, A912,
A913, A914, A915, A916, A917, A918, A919, A920, A921, A922, A923, A924, A925, A926, A927, A928,
A929, A930, A931, A932, A933, A934, A935, A936, A937, A938, A939, A940, A941, A942, A943, A944,
A945, A946, A947, A948, A949, A950, A951, A952, A953, A954, A955, A956, A957, A958, A959, A960,
A961, A962, A963, A964, A965, A966, A967, A968, A969, A970, A971, A972, A973, A974, A975, A976,
A977, A978, A979, A980, A981, A982, A983, A984, A985, A986, A987, A988, A989, A990, A991, A992,
A993, A994, A995, A996, A997, A998, A999
);

#[derive(Default)]
struct M0 {
m1: M1,
m2: M1,
m3: M1,
m4: M1,
m5: M1,
m6: M1,
m7: M1,
m8: M1,
m9: M1,
m10: M1,
m11: M1,
m12: M1,
m13: M1,
m14: M1,
m15: M1,
}

#[derive(Default)]
struct M1 {
m1: M2,
m2: M2,
m3: M2,
m4: M2,
m5: M2,
m6: M2,
m7: M2,
m8: M2,
m9: M2,
m10: M2,
m11: M2,
m12: M2,
m13: M2,
m14: M2,
m15: M2,
}

#[derive(Default)]
struct M2 {
m1: M3,
m2: M3,
m3: M3,
m4: M3,
m5: M3,
m6: M3,
m7: M3,
m8: M3,
m9: M3,
m10: M3,
m11: M3,
m12: M3,
m13: M3,
m14: M3,
m15: M3,
}

#[derive(Default)]
struct M3 {}

fn main() {
M0::default().Au();
//~^ ERROR no method named `Au` found for struct `M0` in the current scope
M0::default().Au();
//~^ ERROR no method named `Au` found for struct `M0` in the current scope
M0::default().Au();
//~^ ERROR no method named `Au` found for struct `M0` in the current scope
M0::default().Au();
//~^ ERROR no method named `Au` found for struct `M0` in the current scope
M0::default().Au();
//~^ ERROR no method named `Au` found for struct `M0` in the current scope
M0::default().Au();
//~^ ERROR no method named `Au` found for struct `M0` in the current scope
M0::default().Au();
//~^ ERROR no method named `Au` found for struct `M0` in the current scope
M0::default().Au();
//~^ ERROR no method named `Au` found for struct `M0` in the current scope
M0::default().Au();
//~^ ERROR no method named `Au` found for struct `M0` in the current scope
}
84 changes: 84 additions & 0 deletions tests/ui/methods/probe-for-diagnostic-doesnt-do-extra-work.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
error[E0599]: no method named `Au` found for struct `M0` in the current scope
--> $DIR/probe-for-diagnostic-doesnt-do-extra-work.rs:145:19
|
LL | struct M0 {
| --------- method `Au` not found for this struct
...
LL | M0::default().Au();
| ^^ method not found in `M0`

error[E0599]: no method named `Au` found for struct `M0` in the current scope
--> $DIR/probe-for-diagnostic-doesnt-do-extra-work.rs:147:19
|
LL | struct M0 {
| --------- method `Au` not found for this struct
...
LL | M0::default().Au();
| ^^ method not found in `M0`

error[E0599]: no method named `Au` found for struct `M0` in the current scope
--> $DIR/probe-for-diagnostic-doesnt-do-extra-work.rs:149:19
|
LL | struct M0 {
| --------- method `Au` not found for this struct
...
LL | M0::default().Au();
| ^^ method not found in `M0`

error[E0599]: no method named `Au` found for struct `M0` in the current scope
--> $DIR/probe-for-diagnostic-doesnt-do-extra-work.rs:151:19
|
LL | struct M0 {
| --------- method `Au` not found for this struct
...
LL | M0::default().Au();
| ^^ method not found in `M0`

error[E0599]: no method named `Au` found for struct `M0` in the current scope
--> $DIR/probe-for-diagnostic-doesnt-do-extra-work.rs:153:19
|
LL | struct M0 {
| --------- method `Au` not found for this struct
...
LL | M0::default().Au();
| ^^ method not found in `M0`

error[E0599]: no method named `Au` found for struct `M0` in the current scope
--> $DIR/probe-for-diagnostic-doesnt-do-extra-work.rs:155:19
|
LL | struct M0 {
| --------- method `Au` not found for this struct
...
LL | M0::default().Au();
| ^^ method not found in `M0`

error[E0599]: no method named `Au` found for struct `M0` in the current scope
--> $DIR/probe-for-diagnostic-doesnt-do-extra-work.rs:157:19
|
LL | struct M0 {
| --------- method `Au` not found for this struct
...
LL | M0::default().Au();
| ^^ method not found in `M0`

error[E0599]: no method named `Au` found for struct `M0` in the current scope
--> $DIR/probe-for-diagnostic-doesnt-do-extra-work.rs:159:19
|
LL | struct M0 {
| --------- method `Au` not found for this struct
...
LL | M0::default().Au();
| ^^ method not found in `M0`

error[E0599]: no method named `Au` found for struct `M0` in the current scope
--> $DIR/probe-for-diagnostic-doesnt-do-extra-work.rs:161:19
|
LL | struct M0 {
| --------- method `Au` not found for this struct
...
LL | M0::default().Au();
| ^^ method not found in `M0`

error: aborting due to 9 previous errors

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