From 70ff7fff47e7825b7f251c1647893f62980b5c9b Mon Sep 17 00:00:00 2001 From: Esteban Kuber Date: Fri, 10 Sep 2021 10:25:22 +0000 Subject: [PATCH 1/4] Restrict parsing of bare union/struct to field types Do not unconditionally parse bare types everywhere, only parse them if they are in a field type, and try them when recovering a misparse everywhere else, using them only if they are successfuly fully parsed. Fix #88583. --- compiler/rustc_parse/src/parser/item.rs | 69 +++++++++++++++-- compiler/rustc_parse/src/parser/ty.rs | 74 ++++++++++++++++--- .../ui/parser/issue-88583-union-as-ident.rs | 15 ++++ 3 files changed, 138 insertions(+), 20 deletions(-) create mode 100644 src/test/ui/parser/issue-88583-union-as-ident.rs diff --git a/compiler/rustc_parse/src/parser/item.rs b/compiler/rustc_parse/src/parser/item.rs index 10c73fd64bc1..c511c22f1988 100644 --- a/compiler/rustc_parse/src/parser/item.rs +++ b/compiler/rustc_parse/src/parser/item.rs @@ -765,12 +765,60 @@ impl<'a> Parser<'a> { if self.eat(&token::Colon) { self.parse_generic_bounds(None)? } else { Vec::new() }; generics.where_clause = self.parse_where_clause()?; - let default = if self.eat(&token::Eq) { Some(self.parse_ty()?) } else { None }; - self.expect_semi()?; + let default = if self.eat(&token::Eq) { + Some(self.parse_ty_recover_anon_adt(|this| this.expect_semi())?.0) + } else { + self.expect_semi()?; + None + }; Ok((ident, ItemKind::TyAlias(Box::new(TyAliasKind(def, generics, bounds, default))))) } + /// Parses a type. If a misparse happens, we attempt to parse the type again, allowing + /// anonymous `union`s and `struct`s in its place. If successful, we return this. The + /// anonymous ADTs will be disallowed elsewhere. + fn parse_ty_recover_anon_adt(&mut self, after: F) -> PResult<'a, (P, R)> + where + F: Fn(&mut Self) -> PResult<'a, R>, + { + if (self.token.is_keyword(kw::Union) | self.token.is_keyword(kw::Struct)) + && self.look_ahead(1, |t| t == &token::OpenDelim(token::Brace)) + { + let mut snapshot = self.clone(); + let mut err; + match self.parse_ty() { + Ok(ty) => match after(self) { + Ok(r) => return Ok((ty, r)), + Err(orig_err) => { + err = orig_err; + } + }, + Err(orig_err) => { + err = orig_err; + } + } + match snapshot.parse_ty_allow_anon_adt() { + Ok(ty) => match after(&mut snapshot) { + Ok(r) => { + err.delay_as_bug(); + *self = snapshot; + return Ok((ty, r)); + } + Err(mut snapshot_err) => snapshot_err.cancel(), + }, + Err(mut snapshot_err) => { + snapshot_err.cancel(); + } + } + Err(err) + } else { + let ty = self.parse_ty()?; + let r = after(self)?; + Ok((ty, r)) + } + } + /// Parses a `UseTree`. /// /// ```text @@ -1059,14 +1107,19 @@ impl<'a> Parser<'a> { // Parse the type of a `const` or `static mut?` item. // That is, the `":" $ty` fragment. - let ty = if self.eat(&token::Colon) { - self.parse_ty()? + let (ty, expr) = if self.eat(&token::Colon) { + self.parse_ty_recover_anon_adt(|this| { + let expr = if this.eat(&token::Eq) { Some(this.parse_expr()?) } else { None }; + this.expect_semi()?; + Ok(expr) + })? } else { - self.recover_missing_const_type(id, m) + let ty = self.recover_missing_const_type(id, m); + let expr = if self.eat(&token::Eq) { Some(self.parse_expr()?) } else { None }; + self.expect_semi()?; + (ty, expr) }; - let expr = if self.eat(&token::Eq) { Some(self.parse_expr()?) } else { None }; - self.expect_semi()?; Ok((id, ty, expr)) } @@ -1438,7 +1491,7 @@ impl<'a> Parser<'a> { ) -> PResult<'a, FieldDef> { let name = self.parse_field_ident(adt_ty, lo)?; self.expect_field_ty_separator()?; - let ty = self.parse_ty()?; + let ty = self.parse_ty_allow_anon_adt()?; if self.token.kind == token::Eq { self.bump(); let const_expr = self.parse_anon_const_expr()?; diff --git a/compiler/rustc_parse/src/parser/ty.rs b/compiler/rustc_parse/src/parser/ty.rs index 299fc916ac97..eb694147965f 100644 --- a/compiler/rustc_parse/src/parser/ty.rs +++ b/compiler/rustc_parse/src/parser/ty.rs @@ -37,6 +37,12 @@ pub(super) enum AllowPlus { No, } +#[derive(Copy, Clone, PartialEq)] +pub(super) enum AllowAnonymousType { + Yes, + No, +} + #[derive(PartialEq)] pub(super) enum RecoverQPath { Yes, @@ -98,9 +104,19 @@ impl<'a> Parser<'a> { AllowCVariadic::No, RecoverQPath::Yes, RecoverReturnSign::Yes, + AllowAnonymousType::No, ) } + pub fn parse_ty_allow_anon_adt(&mut self) -> PResult<'a, P> { + self.parse_ty_common( + AllowPlus::Yes, + AllowCVariadic::No, + RecoverQPath::Yes, + RecoverReturnSign::Yes, + AllowAnonymousType::Yes, + ) + } /// Parse a type suitable for a function or function pointer parameter. /// The difference from `parse_ty` is that this version allows `...` /// (`CVarArgs`) at the top level of the type. @@ -110,6 +126,7 @@ impl<'a> Parser<'a> { AllowCVariadic::Yes, RecoverQPath::Yes, RecoverReturnSign::Yes, + AllowAnonymousType::No, ) } @@ -125,6 +142,7 @@ impl<'a> Parser<'a> { AllowCVariadic::No, RecoverQPath::Yes, RecoverReturnSign::Yes, + AllowAnonymousType::No, ) } @@ -135,6 +153,7 @@ impl<'a> Parser<'a> { AllowCVariadic::Yes, RecoverQPath::Yes, RecoverReturnSign::OnlyFatArrow, + AllowAnonymousType::No, ) } @@ -152,6 +171,7 @@ impl<'a> Parser<'a> { AllowCVariadic::No, recover_qpath, recover_return_sign, + AllowAnonymousType::No, )?; FnRetTy::Ty(ty) } else if recover_return_sign.can_recover(&self.token.kind) { @@ -171,6 +191,7 @@ impl<'a> Parser<'a> { AllowCVariadic::No, recover_qpath, recover_return_sign, + AllowAnonymousType::No, )?; FnRetTy::Ty(ty) } else { @@ -184,6 +205,7 @@ impl<'a> Parser<'a> { allow_c_variadic: AllowCVariadic, recover_qpath: RecoverQPath, recover_return_sign: RecoverReturnSign, + allow_anonymous: AllowAnonymousType, ) -> PResult<'a, P> { let allow_qpath_recovery = recover_qpath == RecoverQPath::Yes; maybe_recover_from_interpolated_ty_qpath!(self, allow_qpath_recovery); @@ -226,19 +248,11 @@ impl<'a> Parser<'a> { } } else if self.eat_keyword(kw::Impl) { self.parse_impl_ty(&mut impl_dyn_multi)? - } else if self.token.is_keyword(kw::Union) + } else if allow_anonymous == AllowAnonymousType::Yes + && (self.token.is_keyword(kw::Union) | self.token.is_keyword(kw::Struct)) && self.look_ahead(1, |t| t == &token::OpenDelim(token::Brace)) { - self.bump(); - let (fields, recovered) = self.parse_record_struct_body("union")?; - let span = lo.to(self.prev_token.span); - self.sess.gated_spans.gate(sym::unnamed_fields, span); - TyKind::AnonymousUnion(fields, recovered) - } else if self.eat_keyword(kw::Struct) { - let (fields, recovered) = self.parse_record_struct_body("struct")?; - let span = lo.to(self.prev_token.span); - self.sess.gated_spans.gate(sym::unnamed_fields, span); - TyKind::AnonymousStruct(fields, recovered) + self.parse_anonymous_ty(lo)? } else if self.is_explicit_dyn_type() { self.parse_dyn_ty(&mut impl_dyn_multi)? } else if self.eat_lt() { @@ -263,7 +277,27 @@ impl<'a> Parser<'a> { let mut err = self.struct_span_err(self.token.span, &msg); err.span_label(self.token.span, "expected type"); self.maybe_annotate_with_ascription(&mut err, true); - return Err(err); + + if allow_anonymous == AllowAnonymousType::No + && (self.token.is_keyword(kw::Union) || self.token.is_keyword(kw::Struct)) + && self.look_ahead(1, |t| t == &token::OpenDelim(token::Brace)) + { + // Recover the parser from anonymous types anywhere other than field types. + let snapshot = self.clone(); + match self.parse_anonymous_ty(lo) { + Ok(ty) => { + err.delay_as_bug(); + ty + } + Err(mut snapshot_err) => { + snapshot_err.cancel(); + *self = snapshot; + return Err(err); + } + } + } else { + return Err(err); + } }; let span = lo.to(self.prev_token.span); @@ -275,6 +309,22 @@ impl<'a> Parser<'a> { self.maybe_recover_from_bad_qpath(ty, allow_qpath_recovery) } + fn parse_anonymous_ty(&mut self, lo: Span) -> PResult<'a, TyKind> { + let is_union = self.token.is_keyword(kw::Union); + self.bump(); + self.parse_record_struct_body(if is_union { "union" } else { "struct" }).map( + |(fields, recovered)| { + let span = lo.to(self.prev_token.span); + self.sess.gated_spans.gate(sym::unnamed_fields, span); + // These will be rejected during AST validation in `deny_anonymous_struct`. + return if is_union { + TyKind::AnonymousUnion(fields, recovered) + } else { + TyKind::AnonymousStruct(fields, recovered) + }; + }, + ) + } /// Parses either: /// - `(TYPE)`, a parenthesized type. /// - `(TYPE,)`, a tuple with a single field of type TYPE. diff --git a/src/test/ui/parser/issue-88583-union-as-ident.rs b/src/test/ui/parser/issue-88583-union-as-ident.rs new file mode 100644 index 000000000000..b3d66d46b1d4 --- /dev/null +++ b/src/test/ui/parser/issue-88583-union-as-ident.rs @@ -0,0 +1,15 @@ +// check-pass + +#![allow(non_camel_case_types)] + +struct union; + +impl union { + pub fn new() -> Self { + union { } + } +} + +fn main() { + let _u = union::new(); +} From 0e989bd5e85e5ce621ac664d1dd065a3425da1df Mon Sep 17 00:00:00 2001 From: Esteban Kuber Date: Fri, 10 Sep 2021 11:48:29 +0000 Subject: [PATCH 2/4] remove `pprint`-check for `type X = union {}` `type X = union {}` isn't supported. Trying to pretty print it will emit a `delay_as_bug` error to be emitted because a later stage should have triggered under normal circumstances. --- src/test/pretty/anonymous-types.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/test/pretty/anonymous-types.rs b/src/test/pretty/anonymous-types.rs index 5ff452e8e43c..983a879ae1f9 100644 --- a/src/test/pretty/anonymous-types.rs +++ b/src/test/pretty/anonymous-types.rs @@ -16,9 +16,4 @@ struct Foo { e: f32, } -type A = - struct { - field: u8, - }; - fn main() { } From 2f7b3d065a1e5b5d327c056d183415dd63cc9ab2 Mon Sep 17 00:00:00 2001 From: Esteban Kuber Date: Fri, 10 Sep 2021 13:35:00 +0000 Subject: [PATCH 3/4] Simplify `parse_ty_recover_anon_adt` It didn't need to be that complex, the `look_ahead` check was enough, as braces aren't allowed after neither `static` or `type`. --- compiler/rustc_parse/src/parser/item.rs | 68 ++++++------------------- 1 file changed, 15 insertions(+), 53 deletions(-) diff --git a/compiler/rustc_parse/src/parser/item.rs b/compiler/rustc_parse/src/parser/item.rs index c511c22f1988..a6f92294b0be 100644 --- a/compiler/rustc_parse/src/parser/item.rs +++ b/compiler/rustc_parse/src/parser/item.rs @@ -765,57 +765,23 @@ impl<'a> Parser<'a> { if self.eat(&token::Colon) { self.parse_generic_bounds(None)? } else { Vec::new() }; generics.where_clause = self.parse_where_clause()?; - let default = if self.eat(&token::Eq) { - Some(self.parse_ty_recover_anon_adt(|this| this.expect_semi())?.0) - } else { - self.expect_semi()?; - None - }; + let default = + if self.eat(&token::Eq) { Some(self.parse_ty_recover_anon_adt()?) } else { None }; + self.expect_semi()?; Ok((ident, ItemKind::TyAlias(Box::new(TyAliasKind(def, generics, bounds, default))))) } - /// Parses a type. If a misparse happens, we attempt to parse the type again, allowing - /// anonymous `union`s and `struct`s in its place. If successful, we return this. The - /// anonymous ADTs will be disallowed elsewhere. - fn parse_ty_recover_anon_adt(&mut self, after: F) -> PResult<'a, (P, R)> - where - F: Fn(&mut Self) -> PResult<'a, R>, - { + /// Parses a type. If we encounter an anonymous `union` or `struct`, we parse it, but still + /// allow `static C: union = union::val;` to be parsed correctly. The anonymous ADTs will be + /// disallowed elsewhere. + fn parse_ty_recover_anon_adt(&mut self) -> PResult<'a, P> { if (self.token.is_keyword(kw::Union) | self.token.is_keyword(kw::Struct)) && self.look_ahead(1, |t| t == &token::OpenDelim(token::Brace)) { - let mut snapshot = self.clone(); - let mut err; - match self.parse_ty() { - Ok(ty) => match after(self) { - Ok(r) => return Ok((ty, r)), - Err(orig_err) => { - err = orig_err; - } - }, - Err(orig_err) => { - err = orig_err; - } - } - match snapshot.parse_ty_allow_anon_adt() { - Ok(ty) => match after(&mut snapshot) { - Ok(r) => { - err.delay_as_bug(); - *self = snapshot; - return Ok((ty, r)); - } - Err(mut snapshot_err) => snapshot_err.cancel(), - }, - Err(mut snapshot_err) => { - snapshot_err.cancel(); - } - } - Err(err) + self.parse_ty_allow_anon_adt() } else { - let ty = self.parse_ty()?; - let r = after(self)?; - Ok((ty, r)) + self.parse_ty() } } @@ -1107,19 +1073,15 @@ impl<'a> Parser<'a> { // Parse the type of a `const` or `static mut?` item. // That is, the `":" $ty` fragment. - let (ty, expr) = if self.eat(&token::Colon) { - self.parse_ty_recover_anon_adt(|this| { - let expr = if this.eat(&token::Eq) { Some(this.parse_expr()?) } else { None }; - this.expect_semi()?; - Ok(expr) - })? + let ty = if self.eat(&token::Colon) { + self.parse_ty_recover_anon_adt()? } else { - let ty = self.recover_missing_const_type(id, m); - let expr = if self.eat(&token::Eq) { Some(self.parse_expr()?) } else { None }; - self.expect_semi()?; - (ty, expr) + self.recover_missing_const_type(id, m) }; + let expr = if self.eat(&token::Eq) { Some(self.parse_expr()?) } else { None }; + self.expect_semi()?; + Ok((id, ty, expr)) } From 191ad9ec3e3c0c47d8cd6254aabc5519466c4c85 Mon Sep 17 00:00:00 2001 From: Esteban Kuber Date: Fri, 10 Sep 2021 13:42:28 +0000 Subject: [PATCH 4/4] make comment more accurate --- compiler/rustc_parse/src/parser/ty.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_parse/src/parser/ty.rs b/compiler/rustc_parse/src/parser/ty.rs index eb694147965f..abf62aeb0dab 100644 --- a/compiler/rustc_parse/src/parser/ty.rs +++ b/compiler/rustc_parse/src/parser/ty.rs @@ -316,7 +316,7 @@ impl<'a> Parser<'a> { |(fields, recovered)| { let span = lo.to(self.prev_token.span); self.sess.gated_spans.gate(sym::unnamed_fields, span); - // These will be rejected during AST validation in `deny_anonymous_struct`. + // These can be rejected during AST validation in `deny_anonymous_struct`. return if is_union { TyKind::AnonymousUnion(fields, recovered) } else {