Skip to content

Commit

Permalink
Auto merge of #107478 - compiler-errors:anon-enum-tys-are-ambiguous, …
Browse files Browse the repository at this point in the history
…r=estebank

Revert "Teach parser to understand fake anonymous enum syntax" and related commits

anonymous enum types are currently ambiguous in positions like:

* `|` operator: `a as fn() -> B | C`
* closure args: `|_: as fn() -> A | B`

I first tried to thread around `RecoverAnonEnum` into all these positions, but the resulting complexity in the compiler is IMO not worth it, or at least worth a bit more thinking time. In the mean time, let's revert this syntax for now, so we can go back to the drawing board.

Fixes #107461

cc: `@estebank` `@cjgillot` #106960

---
### Squashed revert commits:

Revert "review comment: Remove AST AnonTy"

This reverts commit 020cca8.

Revert "Ensure macros are not affected"

This reverts commit 12d18e4.

Revert "Emit fewer errors on patterns with possible type ascription"

This reverts commit c847a01.

Revert "Teach parser to understand fake anonymous enum syntax"

This reverts commit 2d82420.
  • Loading branch information
bors committed Feb 2, 2023
2 parents a9985cf + 39db65c commit 97872b7
Show file tree
Hide file tree
Showing 10 changed files with 72 additions and 265 deletions.
4 changes: 2 additions & 2 deletions compiler/rustc_ast/src/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -403,8 +403,8 @@ pub fn walk_ty<'a, V: Visitor<'a>>(visitor: &mut V, typ: &'a Ty) {
walk_list!(visitor, visit_lifetime, opt_lifetime, LifetimeCtxt::Ref);
visitor.visit_ty(&mutable_type.ty)
}
TyKind::Tup(tys) => {
walk_list!(visitor, visit_ty, tys);
TyKind::Tup(tuple_element_types) => {
walk_list!(visitor, visit_ty, tuple_element_types);
}
TyKind::BareFn(function_declaration) => {
walk_list!(visitor, visit_generic_param, &function_declaration.generic_params);
Expand Down
52 changes: 13 additions & 39 deletions compiler/rustc_parse/src/parser/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2394,7 +2394,7 @@ impl<'a> Parser<'a> {

/// Some special error handling for the "top-level" patterns in a match arm,
/// `for` loop, `let`, &c. (in contrast to subpatterns within such).
pub(crate) fn maybe_recover_colon_colon_in_pat_typo_or_anon_enum(
pub(crate) fn maybe_recover_colon_colon_in_pat_typo(
&mut self,
mut first_pat: P<Pat>,
expected: Option<Expected>,
Expand All @@ -2405,41 +2405,26 @@ impl<'a> Parser<'a> {
if !matches!(first_pat.kind, PatKind::Ident(_, _, None) | PatKind::Path(..))
|| !self.look_ahead(1, |token| token.is_ident() && !token.is_reserved_ident())
{
let mut snapshot_type = self.create_snapshot_for_diagnostic();
snapshot_type.bump(); // `:`
match snapshot_type.parse_ty() {
Err(inner_err) => {
inner_err.cancel();
}
Ok(ty) => {
let Err(mut err) = self.expected_one_of_not_found(&[], &[]) else {
return first_pat;
};
err.span_label(ty.span, "specifying the type of a pattern isn't supported");
self.restore_snapshot(snapshot_type);
let span = first_pat.span.to(ty.span);
first_pat = self.mk_pat(span, PatKind::Wild);
err.emit();
}
}
return first_pat;
}
// The pattern looks like it might be a path with a `::` -> `:` typo:
// `match foo { bar:baz => {} }`
let colon_span = self.token.span;
let span = self.token.span;
// We only emit "unexpected `:`" error here if we can successfully parse the
// whole pattern correctly in that case.
let mut snapshot_pat = self.create_snapshot_for_diagnostic();
let mut snapshot_type = self.create_snapshot_for_diagnostic();
let snapshot = self.create_snapshot_for_diagnostic();

// Create error for "unexpected `:`".
match self.expected_one_of_not_found(&[], &[]) {
Err(mut err) => {
snapshot_pat.bump(); // Skip the `:`.
snapshot_type.bump(); // Skip the `:`.
match snapshot_pat.parse_pat_no_top_alt(expected) {
self.bump(); // Skip the `:`.
match self.parse_pat_no_top_alt(expected) {
Err(inner_err) => {
// Carry on as if we had not done anything, callers will emit a
// reasonable error.
inner_err.cancel();
err.cancel();
self.restore_snapshot(snapshot);
}
Ok(mut pat) => {
// We've parsed the rest of the pattern.
Expand Down Expand Up @@ -2503,33 +2488,22 @@ impl<'a> Parser<'a> {
_ => {}
}
if show_sugg {
err.span_suggestion_verbose(
colon_span.until(self.look_ahead(1, |t| t.span)),
err.span_suggestion(
span,
"maybe write a path separator here",
"::",
Applicability::MaybeIncorrect,
);
} else {
first_pat = self.mk_pat(new_span, PatKind::Wild);
}
self.restore_snapshot(snapshot_pat);
err.emit();
}
}
match snapshot_type.parse_ty() {
Err(inner_err) => {
inner_err.cancel();
}
Ok(ty) => {
err.span_label(ty.span, "specifying the type of a pattern isn't supported");
self.restore_snapshot(snapshot_type);
let new_span = first_pat.span.to(ty.span);
first_pat = self.mk_pat(new_span, PatKind::Wild);
}
}
err.emit();
}
_ => {
// Carry on as if we had not done anything. This should be unreachable.
self.restore_snapshot(snapshot);
}
};
first_pat
Expand Down
3 changes: 1 addition & 2 deletions compiler/rustc_parse/src/parser/pat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,8 +148,7 @@ impl<'a> Parser<'a> {

// Check if the user wrote `foo:bar` instead of `foo::bar`.
if ra == RecoverColon::Yes {
first_pat =
self.maybe_recover_colon_colon_in_pat_typo_or_anon_enum(first_pat, expected);
first_pat = self.maybe_recover_colon_colon_in_pat_typo(first_pat, expected);
}

if let Some(leading_vert_span) = leading_vert_span {
Expand Down
65 changes: 3 additions & 62 deletions compiler/rustc_parse/src/parser/ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ use rustc_ast::{
self as ast, BareFnTy, FnRetTy, GenericBound, GenericBounds, GenericParam, Generics, Lifetime,
MacCall, MutTy, Mutability, PolyTraitRef, TraitBoundModifier, TraitObjectSyntax, Ty, TyKind,
};
use rustc_ast_pretty::pprust;
use rustc_errors::{Applicability, PResult};
use rustc_span::source_map::Span;
use rustc_span::symbol::{kw, sym, Ident};
Expand Down Expand Up @@ -50,24 +49,17 @@ pub(super) enum AllowPlus {
No,
}

#[derive(PartialEq, Clone, Copy)]
#[derive(PartialEq)]
pub(super) enum RecoverQPath {
Yes,
No,
}

#[derive(PartialEq, Clone, Copy)]
pub(super) enum RecoverQuestionMark {
Yes,
No,
}

#[derive(PartialEq, Clone, Copy)]
pub(super) enum RecoverAnonEnum {
Yes,
No,
}

/// Signals whether parsing a type should recover `->`.
///
/// More specifically, when parsing a function like:
Expand Down Expand Up @@ -100,7 +92,7 @@ impl RecoverReturnSign {
}

// Is `...` (`CVarArgs`) legal at this level of type parsing?
#[derive(PartialEq, Clone, Copy)]
#[derive(PartialEq)]
enum AllowCVariadic {
Yes,
No,
Expand All @@ -125,7 +117,6 @@ impl<'a> Parser<'a> {
RecoverReturnSign::Yes,
None,
RecoverQuestionMark::Yes,
RecoverAnonEnum::No,
)
}

Expand All @@ -140,7 +131,6 @@ impl<'a> Parser<'a> {
RecoverReturnSign::Yes,
Some(ty_params),
RecoverQuestionMark::Yes,
RecoverAnonEnum::No,
)
}

Expand All @@ -155,7 +145,6 @@ impl<'a> Parser<'a> {
RecoverReturnSign::Yes,
None,
RecoverQuestionMark::Yes,
RecoverAnonEnum::Yes,
)
}

Expand All @@ -173,7 +162,6 @@ impl<'a> Parser<'a> {
RecoverReturnSign::Yes,
None,
RecoverQuestionMark::Yes,
RecoverAnonEnum::No,
)
}

Expand All @@ -187,7 +175,6 @@ impl<'a> Parser<'a> {
RecoverReturnSign::Yes,
None,
RecoverQuestionMark::No,
RecoverAnonEnum::No,
)
}

Expand All @@ -199,7 +186,6 @@ impl<'a> Parser<'a> {
RecoverReturnSign::Yes,
None,
RecoverQuestionMark::No,
RecoverAnonEnum::No,
)
}

Expand All @@ -212,7 +198,6 @@ impl<'a> Parser<'a> {
RecoverReturnSign::OnlyFatArrow,
None,
RecoverQuestionMark::Yes,
RecoverAnonEnum::No,
)
}

Expand All @@ -232,7 +217,6 @@ impl<'a> Parser<'a> {
recover_return_sign,
None,
RecoverQuestionMark::Yes,
RecoverAnonEnum::Yes,
)?;
FnRetTy::Ty(ty)
} else if recover_return_sign.can_recover(&self.token.kind) {
Expand All @@ -247,7 +231,6 @@ impl<'a> Parser<'a> {
recover_return_sign,
None,
RecoverQuestionMark::Yes,
RecoverAnonEnum::Yes,
)?;
FnRetTy::Ty(ty)
} else {
Expand All @@ -263,7 +246,6 @@ impl<'a> Parser<'a> {
recover_return_sign: RecoverReturnSign,
ty_generics: Option<&Generics>,
recover_question_mark: RecoverQuestionMark,
recover_anon_enum: RecoverAnonEnum,
) -> PResult<'a, P<Ty>> {
let allow_qpath_recovery = recover_qpath == RecoverQPath::Yes;
maybe_recover_from_interpolated_ty_qpath!(self, allow_qpath_recovery);
Expand Down Expand Up @@ -347,50 +329,9 @@ impl<'a> Parser<'a> {
AllowPlus::Yes => self.maybe_recover_from_bad_type_plus(&ty)?,
AllowPlus::No => self.maybe_report_ambiguous_plus(impl_dyn_multi, &ty),
}
if RecoverQuestionMark::Yes == recover_question_mark {
if let RecoverQuestionMark::Yes = recover_question_mark {
ty = self.maybe_recover_from_question_mark(ty);
}
if recover_anon_enum == RecoverAnonEnum::Yes
&& self.check_noexpect(&token::BinOp(token::Or))
&& self.look_ahead(1, |t| t.can_begin_type())
{
let mut pipes = vec![self.token.span];
let mut types = vec![ty];
loop {
if !self.eat(&token::BinOp(token::Or)) {
break;
}
pipes.push(self.prev_token.span);
types.push(self.parse_ty_common(
allow_plus,
allow_c_variadic,
recover_qpath,
recover_return_sign,
ty_generics,
recover_question_mark,
RecoverAnonEnum::No,
)?);
}
let mut err = self.struct_span_err(pipes, "anonymous enums are not supported");
for ty in &types {
err.span_label(ty.span, "");
}
err.help(&format!(
"create a named `enum` and use it here instead:\nenum Name {{\n{}\n}}",
types
.iter()
.enumerate()
.map(|(i, t)| format!(
" Variant{}({}),",
i + 1, // Lets not confuse people with zero-indexing :)
pprust::to_string(|s| s.print_type(&t)),
))
.collect::<Vec<_>>()
.join("\n"),
));
err.emit();
return Ok(self.mk_ty(lo.to(self.prev_token.span), TyKind::Err));
}
if allow_qpath_recovery { self.maybe_recover_from_bad_qpath(ty) } else { Ok(ty) }
}

Expand Down
26 changes: 26 additions & 0 deletions tests/ui/parser/anon-enums-are-ambiguous.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// check-pass

macro_rules! test_expr {
($expr:expr) => {};
}

macro_rules! test_ty {
($a:ty | $b:ty) => {};
}

fn main() {
test_expr!(a as fn() -> B | C);
// Do not break the `|` operator.

test_expr!(|_: fn() -> B| C | D);
// Do not break `-> Ret` in closure args.

test_ty!(A | B);
// We can't support anon enums in arbitrary positions.

test_ty!(fn() -> A | B);
// Don't break fn ptrs.

test_ty!(impl Fn() -> A | B);
// Don't break parenthesized generics.
}
17 changes: 0 additions & 17 deletions tests/ui/parser/anon-enums.rs

This file was deleted.

Loading

0 comments on commit 97872b7

Please sign in to comment.