Skip to content

Teach parser to understand fake anonymous enum syntax #106960

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.

Sign up for GitHub

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 4 commits into from
Jan 26, 2023
Merged

Teach parser to understand fake anonymous enum syntax #106960

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2d82420
Teach parser to understand fake anonymous enum syntax
estebank Jan 15, 2023
c847a01
Emit fewer errors on patterns with possible type ascription
estebank Jan 17, 2023
12d18e4
Ensure macros are not affected
estebank Jan 17, 2023
020cca8
review comment: Remove AST AnonTy
estebank Jan 23, 2023
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
3 changes: 3 additions & 0 deletions compiler/rustc_ast/src/ast.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2096,6 +2096,9 @@ pub enum TyKind {
Err,
/// Placeholder for a `va_list`.
CVarArgs,
/// Placeholder for "anonymous enums", which don't exist, but keeping their
/// information around lets us produce better diagnostics.
AnonEnum(Vec<P<Ty>>),
Copy link
Contributor

Choose a reason for hiding this comment

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

While I generally encourage to add information to the AST for diagnostics, adding a variant that does not exist in the language seems a bit too far.
IIUC, this is only needed for pretty-printing. I couldn't see where it appears in ui tests. Why isn't the usual TyKind::Err sufficient?

Copy link
Contributor Author

@estebank estebank Jan 23, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I can avoid including this for the purposes of this PR, but I have a very early attempt at real support for anonymous enums 1) to have more info for more accurate suggestions in later stages, particularly to deduplicate them (thinking of declaring the same enum in multiple return types) in subsequent work, and 2) having an in-tree proof of concept of the actual feature to convince myself of whether a very restricted version of this would be possible. I can safely remove this from this PR.

Edit: took it out.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe TyKind::Err could contain an enum with parsed invalid syntax, like

enum TyError {
  AnonEnum(Vec<P<Ty>>),
  Other,
}

}

impl TyKind {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_ast/src/mut_visit.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -470,7 +470,7 @@ pub fn noop_visit_ty<T: MutVisitor>(ty: &mut P<Ty>, vis: &mut T) {
vis.visit_fn_decl(decl);
vis.visit_span(decl_span);
}
TyKind::Tup(tys) => visit_vec(tys, |ty| vis.visit_ty(ty)),
TyKind::AnonEnum(tys) | TyKind::Tup(tys) => visit_vec(tys, |ty| vis.visit_ty(ty)),
TyKind::Paren(ty) => vis.visit_ty(ty),
TyKind::Path(qself, path) => {
vis.visit_qself(qself);
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_ast/src/visit.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -400,8 +400,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(tuple_element_types) => {
walk_list!(visitor, visit_ty, tuple_element_types);
TyKind::AnonEnum(tys) | TyKind::Tup(tys) => {
walk_list!(visitor, visit_ty, tys);
}
TyKind::BareFn(function_declaration) => {
walk_list!(visitor, visit_generic_param, &function_declaration.generic_params);
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_ast_lowering/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1235,6 +1235,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
let kind = match &t.kind {
TyKind::Infer => hir::TyKind::Infer,
TyKind::Err => hir::TyKind::Err,
TyKind::AnonEnum(_) => hir::TyKind::Err,
TyKind::Slice(ty) => hir::TyKind::Slice(self.lower_ty(ty, itctx)),
TyKind::Ptr(mt) => hir::TyKind::Ptr(self.lower_mt(mt, itctx)),
TyKind::Ref(region, mt) => {
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_ast_pretty/src/pprust/state.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1041,6 +1041,9 @@ impl<'a> State<'a> {
}
self.pclose();
}
ast::TyKind::AnonEnum(elts) => {
self.strsep("|", false, Inconsistent, elts, |s, ty| s.print_type(ty));
}
ast::TyKind::Paren(typ) => {
self.popen();
self.print_type(typ);
Expand Down
52 changes: 39 additions & 13 deletions compiler/rustc_parse/src/parser/diagnostics.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2372,7 +2372,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(
pub(crate) fn maybe_recover_colon_colon_in_pat_typo_or_anon_enum(
&mut self,
mut first_pat: P<Pat>,
expected: Expected,
Expand All @@ -2383,26 +2383,41 @@ 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 span = self.token.span;
let colon_span = self.token.span;
// We only emit "unexpected `:`" error here if we can successfully parse the
// whole pattern correctly in that case.
let snapshot = self.create_snapshot_for_diagnostic();
let mut snapshot_pat = self.create_snapshot_for_diagnostic();
let mut snapshot_type = self.create_snapshot_for_diagnostic();

// Create error for "unexpected `:`".
match self.expected_one_of_not_found(&[], &[]) {
Err(mut err) => {
self.bump(); // Skip the `:`.
match self.parse_pat_no_top_alt(expected) {
snapshot_pat.bump(); // Skip the `:`.
snapshot_type.bump(); // Skip the `:`.
match snapshot_pat.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 @@ -2466,22 +2481,33 @@ impl<'a> Parser<'a> {
_ => {}
}
if show_sugg {
err.span_suggestion(
span,
err.span_suggestion_verbose(
colon_span.until(self.look_ahead(1, |t| t.span)),
"maybe write a path separator here",
"::",
Applicability::MaybeIncorrect,
);
} else {
first_pat = self.mk_pat(new_span, PatKind::Wild);
}
err.emit();
self.restore_snapshot(snapshot_pat);
}
}
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: 2 additions & 1 deletion compiler/rustc_parse/src/parser/pat.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,8 @@ 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(first_pat, expected);
first_pat =
self.maybe_recover_colon_colon_in_pat_typo_or_anon_enum(first_pat, expected);
}

if let Some(leading_vert_span) = leading_vert_span {
Expand Down
67 changes: 63 additions & 4 deletions compiler/rustc_parse/src/parser/ty.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ 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::{pluralize, struct_span_err, Applicability, PResult};
use rustc_span::source_map::Span;
use rustc_span::symbol::{kw, sym, Ident};
Expand Down Expand Up @@ -43,17 +44,24 @@ pub(super) enum AllowPlus {
No,
}

#[derive(PartialEq)]
#[derive(PartialEq, Clone, Copy)]
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 @@ -86,7 +94,7 @@ impl RecoverReturnSign {
}

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

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

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

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

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

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

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

Expand All @@ -211,6 +226,7 @@ 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 @@ -232,6 +248,7 @@ impl<'a> Parser<'a> {
recover_return_sign,
None,
RecoverQuestionMark::Yes,
RecoverAnonEnum::Yes,
)?;
FnRetTy::Ty(ty)
} else {
Expand All @@ -247,6 +264,7 @@ 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 @@ -325,14 +343,55 @@ impl<'a> Parser<'a> {
let mut ty = self.mk_ty(span, kind);

// Try to recover from use of `+` with incorrect priority.
if matches!(allow_plus, AllowPlus::Yes) {
if allow_plus == AllowPlus::Yes {
self.maybe_recover_from_bad_type_plus(&ty)?;
} else {
self.maybe_report_ambiguous_plus(impl_dyn_multi, &ty);
}
if let RecoverQuestionMark::Yes = recover_question_mark {
if 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::AnonEnum(types)));
}
if allow_qpath_recovery { self.maybe_recover_from_bad_qpath(ty) } else { Ok(ty) }
}

Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_passes/src/hir_stats.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -579,6 +579,7 @@ impl<'v> ast_visit::Visitor<'v> for StatCollector<'v> {
[
Slice,
Array,
AnonEnum,
Ptr,
Ref,
BareFn,
Expand Down
4 changes: 3 additions & 1 deletion src/tools/rustfmt/src/types.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -839,7 +839,9 @@ impl Rewrite for ast::Ty {
})
}
ast::TyKind::CVarArgs => Some("...".to_owned()),
ast::TyKind::Err => Some(context.snippet(self.span).to_owned()),
ast::TyKind::AnonEnum(_) | ast::TyKind::Err => {
Some(context.snippet(self.span).to_owned())
}
ast::TyKind::Typeof(ref anon_const) => rewrite_call(
context,
"typeof",
Expand Down
17 changes: 17 additions & 0 deletions tests/ui/parser/anon-enums.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
fn foo(x: bool | i32) -> i32 | f64 {
//~^ ERROR anonymous enums are not supported
//~| ERROR anonymous enums are not supported
match x {
x: i32 => x, //~ ERROR expected
true => 42.,
false => 0.333,
}
}

fn main() {
match foo(true) {
42: i32 => (), //~ ERROR expected
_: f64 => (), //~ ERROR expected
x: i32 => (), //~ ERROR expected
}
}
Loading