Skip to content

Commit c7d7e37

Browse files
authored
Rollup merge of rust-lang#64909 - estebank:turbofish-reloaded, r=Centril
When encountering chained operators use heuristics to recover from bad turbofish
2 parents ae2a720 + 76456e7 commit c7d7e37

File tree

9 files changed

+286
-86
lines changed

9 files changed

+286
-86
lines changed

Diff for: src/librustc_errors/diagnostic.rs

+68-37
Original file line numberDiff line numberDiff line change
@@ -298,9 +298,31 @@ impl Diagnostic {
298298
/// * may contain a name of a function, variable, or type, but not whole expressions
299299
///
300300
/// See `CodeSuggestion` for more information.
301-
pub fn span_suggestion(&mut self, sp: Span, msg: &str,
302-
suggestion: String,
303-
applicability: Applicability) -> &mut Self {
301+
pub fn span_suggestion(
302+
&mut self,
303+
sp: Span,
304+
msg: &str,
305+
suggestion: String,
306+
applicability: Applicability,
307+
) -> &mut Self {
308+
self.span_suggestion_with_style(
309+
sp,
310+
msg,
311+
suggestion,
312+
applicability,
313+
SuggestionStyle::ShowCode,
314+
);
315+
self
316+
}
317+
318+
pub fn span_suggestion_with_style(
319+
&mut self,
320+
sp: Span,
321+
msg: &str,
322+
suggestion: String,
323+
applicability: Applicability,
324+
style: SuggestionStyle,
325+
) -> &mut Self {
304326
self.suggestions.push(CodeSuggestion {
305327
substitutions: vec![Substitution {
306328
parts: vec![SubstitutionPart {
@@ -309,16 +331,37 @@ impl Diagnostic {
309331
}],
310332
}],
311333
msg: msg.to_owned(),
312-
style: SuggestionStyle::ShowCode,
334+
style,
313335
applicability,
314336
});
315337
self
316338
}
317339

340+
pub fn span_suggestion_verbose(
341+
&mut self,
342+
sp: Span,
343+
msg: &str,
344+
suggestion: String,
345+
applicability: Applicability,
346+
) -> &mut Self {
347+
self.span_suggestion_with_style(
348+
sp,
349+
msg,
350+
suggestion,
351+
applicability,
352+
SuggestionStyle::ShowAlways,
353+
);
354+
self
355+
}
356+
318357
/// Prints out a message with multiple suggested edits of the code.
319-
pub fn span_suggestions(&mut self, sp: Span, msg: &str,
320-
suggestions: impl Iterator<Item = String>, applicability: Applicability) -> &mut Self
321-
{
358+
pub fn span_suggestions(
359+
&mut self,
360+
sp: Span,
361+
msg: &str,
362+
suggestions: impl Iterator<Item = String>,
363+
applicability: Applicability,
364+
) -> &mut Self {
322365
self.suggestions.push(CodeSuggestion {
323366
substitutions: suggestions.map(|snippet| Substitution {
324367
parts: vec![SubstitutionPart {
@@ -340,17 +383,13 @@ impl Diagnostic {
340383
pub fn span_suggestion_short(
341384
&mut self, sp: Span, msg: &str, suggestion: String, applicability: Applicability
342385
) -> &mut Self {
343-
self.suggestions.push(CodeSuggestion {
344-
substitutions: vec![Substitution {
345-
parts: vec![SubstitutionPart {
346-
snippet: suggestion,
347-
span: sp,
348-
}],
349-
}],
350-
msg: msg.to_owned(),
351-
style: SuggestionStyle::HideCodeInline,
386+
self.span_suggestion_with_style(
387+
sp,
388+
msg,
389+
suggestion,
352390
applicability,
353-
});
391+
SuggestionStyle::HideCodeInline,
392+
);
354393
self
355394
}
356395

@@ -363,17 +402,13 @@ impl Diagnostic {
363402
pub fn span_suggestion_hidden(
364403
&mut self, sp: Span, msg: &str, suggestion: String, applicability: Applicability
365404
) -> &mut Self {
366-
self.suggestions.push(CodeSuggestion {
367-
substitutions: vec![Substitution {
368-
parts: vec![SubstitutionPart {
369-
snippet: suggestion,
370-
span: sp,
371-
}],
372-
}],
373-
msg: msg.to_owned(),
374-
style: SuggestionStyle::HideCodeAlways,
405+
self.span_suggestion_with_style(
406+
sp,
407+
msg,
408+
suggestion,
375409
applicability,
376-
});
410+
SuggestionStyle::HideCodeAlways,
411+
);
377412
self
378413
}
379414

@@ -384,17 +419,13 @@ impl Diagnostic {
384419
pub fn tool_only_span_suggestion(
385420
&mut self, sp: Span, msg: &str, suggestion: String, applicability: Applicability
386421
) -> &mut Self {
387-
self.suggestions.push(CodeSuggestion {
388-
substitutions: vec![Substitution {
389-
parts: vec![SubstitutionPart {
390-
snippet: suggestion,
391-
span: sp,
392-
}],
393-
}],
394-
msg: msg.to_owned(),
395-
style: SuggestionStyle::CompletelyHidden,
422+
self.span_suggestion_with_style(
423+
sp,
424+
msg,
425+
suggestion,
396426
applicability,
397-
});
427+
SuggestionStyle::CompletelyHidden,
428+
);
398429
self
399430
}
400431

Diff for: src/librustc_errors/emitter.rs

+8-4
Original file line numberDiff line numberDiff line change
@@ -218,10 +218,14 @@ pub trait Emitter {
218218
sugg.msg.split_whitespace().count() < 10 &&
219219
// don't display multiline suggestions as labels
220220
!sugg.substitutions[0].parts[0].snippet.contains('\n') &&
221-
// when this style is set we want the suggestion to be a message, not inline
222-
sugg.style != SuggestionStyle::HideCodeAlways &&
223-
// trivial suggestion for tooling's sake, never shown
224-
sugg.style != SuggestionStyle::CompletelyHidden
221+
![
222+
// when this style is set we want the suggestion to be a message, not inline
223+
SuggestionStyle::HideCodeAlways,
224+
// trivial suggestion for tooling's sake, never shown
225+
SuggestionStyle::CompletelyHidden,
226+
// subtle suggestion, never shown inline
227+
SuggestionStyle::ShowAlways,
228+
].contains(&sugg.style)
225229
{
226230
let substitution = &sugg.substitutions[0].parts[0].snippet.trim();
227231
let msg = if substitution.len() == 0 || sugg.style.hide_inline() {

Diff for: src/librustc_errors/lib.rs

+2
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,8 @@ pub enum SuggestionStyle {
8181
/// This will *not* show the code if the suggestion is inline *and* the suggested code is
8282
/// empty.
8383
ShowCode,
84+
/// Always show the suggested code independently.
85+
ShowAlways,
8486
}
8587

8688
impl SuggestionStyle {

Diff for: src/libsyntax/parse/diagnostics.rs

+150-13
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ use syntax_pos::{Span, DUMMY_SP, MultiSpan, SpanSnippetError};
1717
use log::{debug, trace};
1818
use std::mem;
1919

20+
const TURBOFISH: &'static str = "use `::<...>` instead of `<...>` to specify type arguments";
2021
/// Creates a placeholder argument.
2122
crate fn dummy_arg(ident: Ident) -> Param {
2223
let pat = P(Pat {
@@ -543,35 +544,154 @@ impl<'a> Parser<'a> {
543544
}
544545

545546
/// Produces an error if comparison operators are chained (RFC #558).
546-
/// We only need to check the LHS, not the RHS, because all comparison ops
547-
/// have same precedence and are left-associative.
548-
crate fn check_no_chained_comparison(&self, lhs: &Expr, outer_op: &AssocOp) -> PResult<'a, ()> {
549-
debug_assert!(outer_op.is_comparison(),
550-
"check_no_chained_comparison: {:?} is not comparison",
551-
outer_op);
547+
/// We only need to check the LHS, not the RHS, because all comparison ops have same
548+
/// precedence (see `fn precedence`) and are left-associative (see `fn fixity`).
549+
///
550+
/// This can also be hit if someone incorrectly writes `foo<bar>()` when they should have used
551+
/// the turbofish (`foo::<bar>()`) syntax. We attempt some heuristic recovery if that is the
552+
/// case.
553+
///
554+
/// Keep in mind that given that `outer_op.is_comparison()` holds and comparison ops are left
555+
/// associative we can infer that we have:
556+
///
557+
/// outer_op
558+
/// / \
559+
/// inner_op r2
560+
/// / \
561+
/// l1 r1
562+
crate fn check_no_chained_comparison(
563+
&mut self,
564+
lhs: &Expr,
565+
outer_op: &AssocOp,
566+
) -> PResult<'a, Option<P<Expr>>> {
567+
debug_assert!(
568+
outer_op.is_comparison(),
569+
"check_no_chained_comparison: {:?} is not comparison",
570+
outer_op,
571+
);
572+
573+
let mk_err_expr = |this: &Self, span| {
574+
Ok(Some(this.mk_expr(span, ExprKind::Err, ThinVec::new())))
575+
};
576+
552577
match lhs.kind {
553578
ExprKind::Binary(op, _, _) if op.node.is_comparison() => {
554579
// Respan to include both operators.
555-
let op_span = op.span.to(self.token.span);
580+
let op_span = op.span.to(self.prev_span);
556581
let mut err = self.struct_span_err(
557582
op_span,
558583
"chained comparison operators require parentheses",
559584
);
585+
586+
let suggest = |err: &mut DiagnosticBuilder<'_>| {
587+
err.span_suggestion_verbose(
588+
op_span.shrink_to_lo(),
589+
TURBOFISH,
590+
"::".to_string(),
591+
Applicability::MaybeIncorrect,
592+
);
593+
};
594+
560595
if op.node == BinOpKind::Lt &&
561596
*outer_op == AssocOp::Less || // Include `<` to provide this recommendation
562597
*outer_op == AssocOp::Greater // even in a case like the following:
563598
{ // Foo<Bar<Baz<Qux, ()>>>
564-
err.help(
565-
"use `::<...>` instead of `<...>` if you meant to specify type arguments");
566-
err.help("or use `(...)` if you meant to specify fn arguments");
567-
// These cases cause too many knock-down errors, bail out (#61329).
568-
return Err(err);
599+
if *outer_op == AssocOp::Less {
600+
let snapshot = self.clone();
601+
self.bump();
602+
// So far we have parsed `foo<bar<`, consume the rest of the type args.
603+
let modifiers = [
604+
(token::Lt, 1),
605+
(token::Gt, -1),
606+
(token::BinOp(token::Shr), -2),
607+
];
608+
self.consume_tts(1, &modifiers[..]);
609+
610+
if !&[
611+
token::OpenDelim(token::Paren),
612+
token::ModSep,
613+
].contains(&self.token.kind) {
614+
// We don't have `foo< bar >(` or `foo< bar >::`, so we rewind the
615+
// parser and bail out.
616+
mem::replace(self, snapshot.clone());
617+
}
618+
}
619+
return if token::ModSep == self.token.kind {
620+
// We have some certainty that this was a bad turbofish at this point.
621+
// `foo< bar >::`
622+
suggest(&mut err);
623+
624+
let snapshot = self.clone();
625+
self.bump(); // `::`
626+
627+
// Consume the rest of the likely `foo<bar>::new()` or return at `foo<bar>`.
628+
match self.parse_expr() {
629+
Ok(_) => {
630+
// 99% certain that the suggestion is correct, continue parsing.
631+
err.emit();
632+
// FIXME: actually check that the two expressions in the binop are
633+
// paths and resynthesize new fn call expression instead of using
634+
// `ExprKind::Err` placeholder.
635+
mk_err_expr(self, lhs.span.to(self.prev_span))
636+
}
637+
Err(mut expr_err) => {
638+
expr_err.cancel();
639+
// Not entirely sure now, but we bubble the error up with the
640+
// suggestion.
641+
mem::replace(self, snapshot);
642+
Err(err)
643+
}
644+
}
645+
} else if token::OpenDelim(token::Paren) == self.token.kind {
646+
// We have high certainty that this was a bad turbofish at this point.
647+
// `foo< bar >(`
648+
suggest(&mut err);
649+
// Consume the fn call arguments.
650+
match self.consume_fn_args() {
651+
Err(()) => Err(err),
652+
Ok(()) => {
653+
err.emit();
654+
// FIXME: actually check that the two expressions in the binop are
655+
// paths and resynthesize new fn call expression instead of using
656+
// `ExprKind::Err` placeholder.
657+
mk_err_expr(self, lhs.span.to(self.prev_span))
658+
}
659+
}
660+
} else {
661+
// All we know is that this is `foo < bar >` and *nothing* else. Try to
662+
// be helpful, but don't attempt to recover.
663+
err.help(TURBOFISH);
664+
err.help("or use `(...)` if you meant to specify fn arguments");
665+
// These cases cause too many knock-down errors, bail out (#61329).
666+
Err(err)
667+
};
569668
}
570669
err.emit();
571670
}
572671
_ => {}
573672
}
574-
Ok(())
673+
Ok(None)
674+
}
675+
676+
fn consume_fn_args(&mut self) -> Result<(), ()> {
677+
let snapshot = self.clone();
678+
self.bump(); // `(`
679+
680+
// Consume the fn call arguments.
681+
let modifiers = [
682+
(token::OpenDelim(token::Paren), 1),
683+
(token::CloseDelim(token::Paren), -1),
684+
];
685+
self.consume_tts(1, &modifiers[..]);
686+
687+
if self.token.kind == token::Eof {
688+
// Not entirely sure that what we consumed were fn arguments, rollback.
689+
mem::replace(self, snapshot);
690+
Err(())
691+
} else {
692+
// 99% certain that the suggestion is correct, continue parsing.
693+
Ok(())
694+
}
575695
}
576696

577697
crate fn maybe_report_ambiguous_plus(
@@ -1364,6 +1484,23 @@ impl<'a> Parser<'a> {
13641484
err
13651485
}
13661486

1487+
fn consume_tts(
1488+
&mut self,
1489+
mut acc: i64, // `i64` because malformed code can have more closing delims than opening.
1490+
// Not using `FxHashMap` due to `token::TokenKind: !Eq + !Hash`.
1491+
modifier: &[(token::TokenKind, i64)],
1492+
) {
1493+
while acc > 0 {
1494+
if let Some((_, val)) = modifier.iter().find(|(t, _)| *t == self.token.kind) {
1495+
acc += *val;
1496+
}
1497+
if self.token.kind == token::Eof {
1498+
break;
1499+
}
1500+
self.bump();
1501+
}
1502+
}
1503+
13671504
/// Replace duplicated recovered parameters with `_` pattern to avoid unecessary errors.
13681505
///
13691506
/// This is necessary because at this point we don't know whether we parsed a function with

Diff for: src/libsyntax/parse/parser/expr.rs

+3-1
Original file line numberDiff line numberDiff line change
@@ -238,7 +238,9 @@ impl<'a> Parser<'a> {
238238

239239
self.bump();
240240
if op.is_comparison() {
241-
self.check_no_chained_comparison(&lhs, &op)?;
241+
if let Some(expr) = self.check_no_chained_comparison(&lhs, &op)? {
242+
return Ok(expr);
243+
}
242244
}
243245
// Special cases:
244246
if op == AssocOp::As {

0 commit comments

Comments
 (0)