From 5233bc91da2f48f5bc79c688b5cfad846a8127da Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 14 Feb 2024 14:50:49 +1100 Subject: [PATCH] Add an `ErrorGuaranteed` to `ast::TyKind::Err`. This makes it more like `hir::TyKind::Err`, and avoids a `span_delayed_bug` call in `LoweringContext::lower_ty_direct`. It also requires adding `ast::TyKind::Dummy`, now that `ast::TyKind::Err` can't be used for that purpose in the absence of an error emission. There are a couple of cases that aren't as neat as I would have liked, marked with `FIXME` comments. --- compiler/rustc_ast/src/ast.rs | 6 +++-- compiler/rustc_ast/src/mut_visit.rs | 9 +++++-- compiler/rustc_ast/src/visit.rs | 2 +- compiler/rustc_ast_lowering/src/lib.rs | 3 ++- .../rustc_ast_passes/src/ast_validation.rs | 3 ++- compiler/rustc_ast_pretty/src/pprust/state.rs | 7 ++++- compiler/rustc_expand/src/base.rs | 9 ++++--- .../rustc_parse/src/parser/diagnostics.rs | 20 +++++++------- compiler/rustc_parse/src/parser/item.rs | 26 +++++++++++++++---- compiler/rustc_parse/src/parser/path.rs | 7 ++--- compiler/rustc_parse/src/parser/ty.rs | 10 ++++--- compiler/rustc_passes/src/hir_stats.rs | 5 ++-- .../clippy/clippy_utils/src/ast_utils.rs | 2 +- src/tools/rustfmt/src/types.rs | 2 +- 14 files changed, 75 insertions(+), 36 deletions(-) diff --git a/compiler/rustc_ast/src/ast.rs b/compiler/rustc_ast/src/ast.rs index 098e2606a3be9..08fc8e3c6cca8 100644 --- a/compiler/rustc_ast/src/ast.rs +++ b/compiler/rustc_ast/src/ast.rs @@ -2136,10 +2136,12 @@ pub enum TyKind { ImplicitSelf, /// A macro in the type position. MacCall(P), - /// Placeholder for a kind that has failed to be defined. - Err, /// Placeholder for a `va_list`. CVarArgs, + /// Sometimes we need a dummy value when no error has occurred. + Dummy, + /// Placeholder for a kind that has failed to be defined. + Err(ErrorGuaranteed), } impl TyKind { diff --git a/compiler/rustc_ast/src/mut_visit.rs b/compiler/rustc_ast/src/mut_visit.rs index d482ada170ee3..c42c41999732c 100644 --- a/compiler/rustc_ast/src/mut_visit.rs +++ b/compiler/rustc_ast/src/mut_visit.rs @@ -481,7 +481,12 @@ pub fn noop_visit_ty(ty: &mut P, vis: &mut T) { let Ty { id, kind, span, tokens } = ty.deref_mut(); vis.visit_id(id); match kind { - TyKind::Infer | TyKind::ImplicitSelf | TyKind::Err | TyKind::Never | TyKind::CVarArgs => {} + TyKind::Infer + | TyKind::ImplicitSelf + | TyKind::Err(_) + | TyKind::Dummy + | TyKind::Never + | TyKind::CVarArgs => {} TyKind::Slice(ty) => vis.visit_ty(ty), TyKind::Ptr(mt) => vis.visit_mt(mt), TyKind::Ref(lt, mt) => { @@ -1649,7 +1654,7 @@ impl DummyAstNode for Ty { fn dummy() -> Self { Ty { id: DUMMY_NODE_ID, - kind: TyKind::Err, + kind: TyKind::Dummy, span: Default::default(), tokens: Default::default(), } diff --git a/compiler/rustc_ast/src/visit.rs b/compiler/rustc_ast/src/visit.rs index 4aaaa0ba42457..83f6746bdeb23 100644 --- a/compiler/rustc_ast/src/visit.rs +++ b/compiler/rustc_ast/src/visit.rs @@ -447,7 +447,7 @@ pub fn walk_ty<'a, V: Visitor<'a>>(visitor: &mut V, typ: &'a Ty) { walk_list!(visitor, visit_param_bound, bounds, BoundKind::Impl); } TyKind::Typeof(expression) => visitor.visit_anon_const(expression), - TyKind::Infer | TyKind::ImplicitSelf | TyKind::Err => {} + TyKind::Infer | TyKind::ImplicitSelf | TyKind::Dummy | TyKind::Err(_) => {} TyKind::MacCall(mac) => visitor.visit_mac_call(mac), TyKind::Never | TyKind::CVarArgs => {} TyKind::AnonStruct(_, ref fields) | TyKind::AnonUnion(_, ref fields) => { diff --git a/compiler/rustc_ast_lowering/src/lib.rs b/compiler/rustc_ast_lowering/src/lib.rs index 6b5fc01424085..c1b7af94b02a0 100644 --- a/compiler/rustc_ast_lowering/src/lib.rs +++ b/compiler/rustc_ast_lowering/src/lib.rs @@ -1285,7 +1285,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { fn lower_ty_direct(&mut self, t: &Ty, itctx: ImplTraitContext) -> hir::Ty<'hir> { let kind = match &t.kind { TyKind::Infer => hir::TyKind::Infer, - TyKind::Err => hir::TyKind::Err(self.dcx().has_errors().unwrap()), + TyKind::Err(guar) => hir::TyKind::Err(*guar), // Lower the anonymous structs or unions in a nested lowering context. // // ``` @@ -1503,6 +1503,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { ); hir::TyKind::Err(guar) } + TyKind::Dummy => panic!("`TyKind::Dummy` should never be lowered"), }; hir::Ty { kind, span: self.lower_span(t.span), hir_id: self.lower_node_id(t.id) } diff --git a/compiler/rustc_ast_passes/src/ast_validation.rs b/compiler/rustc_ast_passes/src/ast_validation.rs index ebc5c17a4c4f2..fa0f532619611 100644 --- a/compiler/rustc_ast_passes/src/ast_validation.rs +++ b/compiler/rustc_ast_passes/src/ast_validation.rs @@ -881,7 +881,8 @@ impl<'a> Visitor<'a> for AstValidator<'a> { &item.vis, errors::VisibilityNotPermittedNote::TraitImpl, ); - if let TyKind::Err = self_ty.kind { + // njn: use Dummy here + if let TyKind::Err(_) = self_ty.kind { this.dcx().emit_err(errors::ObsoleteAuto { span: item.span }); } if let (&Unsafe::Yes(span), &ImplPolarity::Negative(sp)) = (unsafety, polarity) diff --git a/compiler/rustc_ast_pretty/src/pprust/state.rs b/compiler/rustc_ast_pretty/src/pprust/state.rs index cda746894e865..480506272d2ba 100644 --- a/compiler/rustc_ast_pretty/src/pprust/state.rs +++ b/compiler/rustc_ast_pretty/src/pprust/state.rs @@ -1048,11 +1048,16 @@ impl<'a> State<'a> { ast::TyKind::Infer => { self.word("_"); } - ast::TyKind::Err => { + ast::TyKind::Err(_) => { self.popen(); self.word("/*ERROR*/"); self.pclose(); } + ast::TyKind::Dummy => { + self.popen(); + self.word("/*DUMMY*/"); + self.pclose(); + } ast::TyKind::ImplicitSelf => { self.word("Self"); } diff --git a/compiler/rustc_expand/src/base.rs b/compiler/rustc_expand/src/base.rs index cfeb31fc4c813..20cfc72f6d362 100644 --- a/compiler/rustc_expand/src/base.rs +++ b/compiler/rustc_expand/src/base.rs @@ -567,10 +567,13 @@ impl DummyResult { } /// A plain dummy type. - pub fn raw_ty(sp: Span, is_error: bool) -> P { + pub fn raw_ty(sp: Span) -> P { + // FIXME(nnethercote): you might expect `ast::TyKind::Dummy` to be used here, but some + // values produced here end up being lowered to HIR, which `ast::TyKind::Dummy` does not + // support, so we use an empty tuple instead. P(ast::Ty { id: ast::DUMMY_NODE_ID, - kind: if is_error { ast::TyKind::Err } else { ast::TyKind::Tup(ThinVec::new()) }, + kind: ast::TyKind::Tup(ThinVec::new()), span: sp, tokens: None, }) @@ -611,7 +614,7 @@ impl MacResult for DummyResult { } fn make_ty(self: Box) -> Option> { - Some(DummyResult::raw_ty(self.span, self.is_error)) + Some(DummyResult::raw_ty(self.span)) } fn make_arms(self: Box) -> Option> { diff --git a/compiler/rustc_parse/src/parser/diagnostics.rs b/compiler/rustc_parse/src/parser/diagnostics.rs index 445d5b2ce790c..69518287f38ec 100644 --- a/compiler/rustc_parse/src/parser/diagnostics.rs +++ b/compiler/rustc_parse/src/parser/diagnostics.rs @@ -46,14 +46,14 @@ use std::ops::{Deref, DerefMut}; use thin_vec::{thin_vec, ThinVec}; /// Creates a placeholder argument. -pub(super) fn dummy_arg(ident: Ident) -> Param { +pub(super) fn dummy_arg(ident: Ident, guar: ErrorGuaranteed) -> Param { let pat = P(Pat { id: ast::DUMMY_NODE_ID, kind: PatKind::Ident(BindingAnnotation::NONE, ident, None), span: ident.span, tokens: None, }); - let ty = Ty { kind: TyKind::Err, span: ident.span, id: ast::DUMMY_NODE_ID, tokens: None }; + let ty = Ty { kind: TyKind::Err(guar), span: ident.span, id: ast::DUMMY_NODE_ID, tokens: None }; Param { attrs: AttrVec::default(), id: ast::DUMMY_NODE_ID, @@ -1540,14 +1540,14 @@ impl<'a> Parser<'a> { pub(super) fn maybe_recover_from_question_mark(&mut self, ty: P) -> P { if self.token == token::Question { self.bump(); - self.dcx().emit_err(QuestionMarkInType { + let guar = self.dcx().emit_err(QuestionMarkInType { span: self.prev_token.span, sugg: QuestionMarkInTypeSugg { left: ty.span.shrink_to_lo(), right: self.prev_token.span, }, }); - self.mk_ty(ty.span.to(self.prev_token.span), TyKind::Err) + self.mk_ty(ty.span.to(self.prev_token.span), TyKind::Err(guar)) } else { ty } @@ -2304,8 +2304,8 @@ impl<'a> Parser<'a> { pub(super) fn recover_bad_self_param(&mut self, mut param: Param) -> PResult<'a, Param> { let span = param.pat.span; - param.ty.kind = TyKind::Err; - self.dcx().emit_err(SelfParamNotFirst { span }); + let guar = self.dcx().emit_err(SelfParamNotFirst { span }); + param.ty.kind = TyKind::Err(guar); Ok(param) } @@ -2437,7 +2437,7 @@ impl<'a> Parser<'a> { pub(super) fn deduplicate_recovered_params_names(&self, fn_inputs: &mut ThinVec) { let mut seen_inputs = FxHashSet::default(); for input in fn_inputs.iter_mut() { - let opt_ident = if let (PatKind::Ident(_, ident, _), TyKind::Err) = + let opt_ident = if let (PatKind::Ident(_, ident, _), TyKind::Err(_)) = (&input.pat.kind, &input.ty.kind) { Some(*ident) @@ -2644,8 +2644,10 @@ impl<'a> Parser<'a> { "::", Applicability::MaybeIncorrect, ); - err.emit(); - return Ok(GenericArg::Type(self.mk_ty(start.to(expr.span), TyKind::Err))); + let guar = err.emit(); + return Ok(GenericArg::Type( + self.mk_ty(start.to(expr.span), TyKind::Err(guar)), + )); } else if token::Comma == self.token.kind || self.token.kind.should_end_const_arg() { // Avoid the following output by checking that we consumed a full const arg: diff --git a/compiler/rustc_parse/src/parser/item.rs b/compiler/rustc_parse/src/parser/item.rs index 8050b34956ce9..a0605b8a44c3a 100644 --- a/compiler/rustc_parse/src/parser/item.rs +++ b/compiler/rustc_parse/src/parser/item.rs @@ -591,7 +591,23 @@ impl<'a> Parser<'a> { let ty_second = if self.token == token::DotDot { // We need to report this error after `cfg` expansion for compatibility reasons self.bump(); // `..`, do not add it to expected tokens - Some(self.mk_ty(self.prev_token.span, TyKind::Err)) + + // FIXME(nnethercote): AST validation later detects this + // `TyKind::Err` and emits an errors. So why the unchecked + // ErrorGuaranteed? + // - A `span_delayed_bug` doesn't work here, because rustfmt can + // hit this path but then not hit the follow-up path in the AST + // validator that issues the error, which results in ICEs. + // - `TyKind::Dummy` doesn't work, because it ends up reaching HIR + // lowering, which results in ICEs. Changing `TyKind::Dummy` to + // `TyKind::Err` during AST validation might fix that, but that's + // not possible because AST validation doesn't allow mutability. + // + // #121072 will hopefully remove all this special handling of the + // obsolete `impl Trait for ..` and then this can go away. + #[allow(deprecated)] + let guar = rustc_errors::ErrorGuaranteed::unchecked_error_guaranteed(); + Some(self.mk_ty(self.prev_token.span, TyKind::Err(guar))) } else if has_for || self.token.can_begin_type() { Some(self.parse_ty()?) } else { @@ -2628,13 +2644,13 @@ impl<'a> Parser<'a> { p.recover_diff_marker(); let snapshot = p.create_snapshot_for_diagnostic(); let param = p.parse_param_general(req_name, first_param).or_else(|e| { - e.emit(); + let guar = e.emit(); let lo = p.prev_token.span; p.restore_snapshot(snapshot); // Skip every token until next possible arg or end. p.eat_to_tokens(&[&token::Comma, &token::CloseDelim(Delimiter::Parenthesis)]); // Create a placeholder argument for proper arg count (issue #34264). - Ok(dummy_arg(Ident::new(kw::Empty, lo.to(p.prev_token.span)))) + Ok(dummy_arg(Ident::new(kw::Empty, lo.to(p.prev_token.span)), guar)) }); // ...now that we've parsed the first argument, `self` is no longer allowed. first_param = false; @@ -2671,8 +2687,8 @@ impl<'a> Parser<'a> { return if let Some(ident) = this.parameter_without_type(&mut err, pat, is_name_required, first_param) { - err.emit(); - Ok((dummy_arg(ident), TrailingToken::None)) + let guar = err.emit(); + Ok((dummy_arg(ident, guar), TrailingToken::None)) } else { Err(err) }; diff --git a/compiler/rustc_parse/src/parser/path.rs b/compiler/rustc_parse/src/parser/path.rs index e7cad74b4dd14..681039999a652 100644 --- a/compiler/rustc_parse/src/parser/path.rs +++ b/compiler/rustc_parse/src/parser/path.rs @@ -678,8 +678,9 @@ impl<'a> Parser<'a> { c.into() } Some(GenericArg::Lifetime(lt)) => { - self.dcx().emit_err(errors::AssocLifetime { span, lifetime: lt.ident.span }); - self.mk_ty(span, ast::TyKind::Err).into() + let guar = + self.dcx().emit_err(errors::AssocLifetime { span, lifetime: lt.ident.span }); + self.mk_ty(span, ast::TyKind::Err(guar)).into() } None => { let after_eq = eq.shrink_to_hi(); @@ -779,7 +780,7 @@ impl<'a> Parser<'a> { // type to determine if error recovery has occurred and if the input is not a // syntactically valid type after all. if let ast::TyKind::Slice(inner_ty) | ast::TyKind::Array(inner_ty, _) = &ty.kind - && let ast::TyKind::Err = inner_ty.kind + && let ast::TyKind::Err(_) = inner_ty.kind && let Some(snapshot) = snapshot && let Some(expr) = self.recover_unbraced_const_arg_that_can_begin_ty(snapshot) diff --git a/compiler/rustc_parse/src/parser/ty.rs b/compiler/rustc_parse/src/parser/ty.rs index 157fb9e505af7..f79f2a813b223 100644 --- a/compiler/rustc_parse/src/parser/ty.rs +++ b/compiler/rustc_parse/src/parser/ty.rs @@ -346,8 +346,10 @@ impl<'a> Parser<'a> { AllowCVariadic::No => { // FIXME(Centril): Should we just allow `...` syntactically // anywhere in a type and use semantic restrictions instead? - self.dcx().emit_err(NestedCVariadicType { span: lo.to(self.prev_token.span) }); - TyKind::Err + let guar = self + .dcx() + .emit_err(NestedCVariadicType { span: lo.to(self.prev_token.span) }); + TyKind::Err(guar) } } } else { @@ -493,8 +495,8 @@ impl<'a> Parser<'a> { { // Recover from `[LIT; EXPR]` and `[LIT]` self.bump(); - err.emit(); - self.mk_ty(self.prev_token.span, TyKind::Err) + let guar = err.emit(); + self.mk_ty(self.prev_token.span, TyKind::Err(guar)) } Err(err) => return Err(err), }; diff --git a/compiler/rustc_passes/src/hir_stats.rs b/compiler/rustc_passes/src/hir_stats.rs index d02e86dd45697..96429bb778882 100644 --- a/compiler/rustc_passes/src/hir_stats.rs +++ b/compiler/rustc_passes/src/hir_stats.rs @@ -616,8 +616,9 @@ impl<'v> ast_visit::Visitor<'v> for StatCollector<'v> { Infer, ImplicitSelf, MacCall, - Err, - CVarArgs + CVarArgs, + Dummy, + Err ] ); diff --git a/src/tools/clippy/clippy_utils/src/ast_utils.rs b/src/tools/clippy/clippy_utils/src/ast_utils.rs index adc35bd82ae39..0467a8a65709a 100644 --- a/src/tools/clippy/clippy_utils/src/ast_utils.rs +++ b/src/tools/clippy/clippy_utils/src/ast_utils.rs @@ -690,7 +690,7 @@ pub fn eq_ty(l: &Ty, r: &Ty) -> bool { match (&l.kind, &r.kind) { (Paren(l), _) => eq_ty(l, r), (_, Paren(r)) => eq_ty(l, r), - (Never, Never) | (Infer, Infer) | (ImplicitSelf, ImplicitSelf) | (Err, Err) | (CVarArgs, CVarArgs) => true, + (Never, Never) | (Infer, Infer) | (ImplicitSelf, ImplicitSelf) | (Err(_), Err(_)) | (CVarArgs, CVarArgs) => true, (Slice(l), Slice(r)) => eq_ty(l, r), (Array(le, ls), Array(re, rs)) => eq_ty(le, re) && eq_expr(&ls.value, &rs.value), (Ptr(l), Ptr(r)) => l.mutbl == r.mutbl && eq_ty(&l.ty, &r.ty), diff --git a/src/tools/rustfmt/src/types.rs b/src/tools/rustfmt/src/types.rs index 4cd8e6a703e53..7f220a456a8ee 100644 --- a/src/tools/rustfmt/src/types.rs +++ b/src/tools/rustfmt/src/types.rs @@ -859,7 +859,7 @@ impl Rewrite for ast::Ty { }) } ast::TyKind::CVarArgs => Some("...".to_owned()), - ast::TyKind::Err => Some(context.snippet(self.span).to_owned()), + ast::TyKind::Dummy | ast::TyKind::Err(_) => Some(context.snippet(self.span).to_owned()), ast::TyKind::Typeof(ref anon_const) => rewrite_call( context, "typeof",