Skip to content
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.

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

[Proof of concept] Recover from struct literals with placeholder path #122288

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions compiler/rustc_errors/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -507,6 +507,7 @@ struct DiagCtxtInner {
#[derive(Copy, Clone, PartialEq, Eq, Hash)]
pub enum StashKey {
ItemNoType,
StructLitNoType,
UnderscoreForArrayLengths,
EarlySyntaxWarning,
CallIntoMethod,
Expand Down
34 changes: 34 additions & 0 deletions compiler/rustc_hir_typeck/src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1640,6 +1640,40 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
fields: &'tcx [hir::ExprField<'tcx>],
base_expr: &'tcx Option<&'tcx hir::Expr<'tcx>>,
) -> Ty<'tcx> {
// FIXME(fmease): Move this into separate method.
// FIXME(fmease): This doesn't get called given `(_ { x: () }).x` (`hir::Field`).
// Figure out why.
if let QPath::Resolved(None, hir::Path { res: Res::Err, segments, .. }) = qpath
&& let [segment] = segments
&& segment.ident.name == kw::Empty
&& let Expectation::ExpectHasType(ty) = expected
&& ty.is_adt()
&& let Some(guar) = self.dcx().try_steal_modify_and_emit_err(
expr.span,
StashKey::StructLitNoType,
|err| {
// The parser provided a sub-optimal `HasPlaceholders` suggestion for the type.
// We are typeck and have the real type, so remove that and suggest the actual type.
if let Ok(suggestions) = &mut err.suggestions {
suggestions.clear();
}

err.span_suggestion(
qpath.span(),
// FIXME(fmease): Make this translatable.
"replace it with the correct type",
// FIXME(fmease): This doesn't qualify paths within the type appropriately.
// FIXME(fmease): This doesn't use turbofish when emitting generic args.
// FIXME(fmease): Make the type suggestable.
ty.to_string(),
Applicability::MaybeIncorrect,
);
},
)
{
return Ty::new_error(self.tcx, guar);
}

// Find the relevant variant
let (variant, adt_ty) = match self.check_struct_path(qpath, expr.hir_id) {
Ok(data) => data,
Expand Down
5 changes: 5 additions & 0 deletions compiler/rustc_parse/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -708,6 +708,11 @@ parse_struct_literal_needing_parens =
parse_struct_literal_not_allowed_here = struct literals are not allowed here
.suggestion = surround the struct literal with parentheses

parse_struct_literal_placeholder_path =
the placeholder `_` is not allowed for the path in struct literals
.label = not allowed in struct literals
.suggestion = replace it with an appropriate type

parse_suffixed_literal_in_attribute = suffixed literals are not allowed in attributes
.help = instead of using a suffixed literal (`1u8`, `1.0f32`, etc.), use an unsuffixed version (`1`, `1.0`, etc.)

Expand Down
9 changes: 9 additions & 0 deletions compiler/rustc_parse/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2974,3 +2974,12 @@ pub(crate) struct AsyncImpl {
#[primary_span]
pub span: Span,
}

#[derive(Diagnostic)]
#[diag(parse_struct_literal_placeholder_path)]
pub(crate) struct StructLiteralPlaceholderPath {
#[primary_span]
#[label]
#[suggestion(applicability = "has-placeholders", code = "/*Type*/")]
pub span: Span,
}
4 changes: 4 additions & 0 deletions compiler/rustc_parse/src/parser/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -951,6 +951,10 @@ impl<'a> Parser<'a> {
return None;
}
} else {
// FIXME(fmease): Under certain conditions return a struct literal
// here and stash this diagnostic under StructLitNoType.
// FIXME(fmease): Furthermore don't suggest redundant curly braces
// around the potential struct literal if possible.
self.dcx().emit_err(StructLiteralBodyWithoutPath {
span: expr.span,
sugg: StructLiteralBodyWithoutPathSugg {
Expand Down
26 changes: 26 additions & 0 deletions compiler/rustc_parse/src/parser/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1505,6 +1505,10 @@ impl<'a> Parser<'a> {
} else if this.check_keyword(kw::Let) {
this.parse_expr_let(restrictions)
} else if this.eat_keyword(kw::Underscore) {
if let Some(expr) = this.maybe_recover_bad_struct_literal_path()? {
return Ok(expr);
}

Ok(this.mk_expr(this.prev_token.span, ExprKind::Underscore))
} else if this.token.uninterpolated_span().at_least_rust_2018() {
// `Span::at_least_rust_2018()` is somewhat expensive; don't get it repeatedly.
Expand Down Expand Up @@ -3720,6 +3724,28 @@ impl<'a> Parser<'a> {
});
}

fn maybe_recover_bad_struct_literal_path(&mut self) -> PResult<'a, Option<P<Expr>>> {
if self.may_recover()
&& self.check_noexpect(&token::OpenDelim(Delimiter::Brace))
&& (!self.restrictions.contains(Restrictions::NO_STRUCT_LITERAL)
|| self.is_certainly_not_a_block())
{
let span = self.prev_token.span;
self.bump();

let expr =
self.parse_expr_struct(None, Path::from_ident(Ident::new(kw::Empty, span)), false)?;

self.dcx()
.create_err(errors::StructLiteralPlaceholderPath { span: expr.span })
.stash(expr.span, StashKey::StructLitNoType);

Ok(Some(expr))
} else {
Ok(None)
}
}

fn err_dotdotdot_syntax(&self, span: Span) {
self.dcx().emit_err(errors::DotDotDot { span });
}
Expand Down
7 changes: 7 additions & 0 deletions compiler/rustc_resolve/src/late.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4304,6 +4304,13 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {
}

ExprKind::Struct(ref se) => {
// FIXME(fmease): Add a comment maybe.
if se.path == kw::Empty
&& self.r.dcx().has_stashed_diagnostic(expr.span, StashKey::StructLitNoType)
{
return;
Copy link
Member Author

Choose a reason for hiding this comment

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

We're skipping name resolution entirely as mentioned in the PR description.
@petrochenkov, I'm sure this is super wrong. Please do enlighten me how I
could do better. No rush, though, this is a low priority experimental diagnostic PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

Do pings actually work inside draft PRs? 🤔

}

self.smart_resolve_path(expr.id, &se.qself, &se.path, PathSource::Struct);
// This is the same as `visit::walk_expr(self, expr);`, but we want to pass the
// parent in for accurate suggestions when encountering `Foo { bar }` that should
Expand Down
21 changes: 21 additions & 0 deletions tests/ui/suggestions/struct-lit-placeholder-path.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// Regression test for issue #98282.

mod blah {
pub struct Stuff { x: i32 }
pub fn do_stuff(_: Stuff) {}
}

fn main() {
blah::do_stuff(_ { x: 10 });
//~^ ERROR the placeholder `_` is not allowed for the path in struct literals
//~| NOTE not allowed in struct literals
//~| HELP replace it with the correct type
}

#[cfg(FALSE)]
fn disabled() {
blah::do_stuff(_ { x: 10 });
//~^ ERROR the placeholder `_` is not allowed for the path in struct literals
//~| NOTE not allowed in struct literals
//~| HELP replace it with an appropriate type
}
20 changes: 20 additions & 0 deletions tests/ui/suggestions/struct-lit-placeholder-path.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
error: the placeholder `_` is not allowed for the path in struct literals
--> $DIR/struct-lit-placeholder-path.rs:9:20
|
LL | blah::do_stuff(_ { x: 10 });
| -^^^^^^^^^^
| |
| not allowed in struct literals
| help: replace it with the correct type: `Stuff`

error: the placeholder `_` is not allowed for the path in struct literals
--> $DIR/struct-lit-placeholder-path.rs:17:20
|
LL | blah::do_stuff(_ { x: 10 });
| ^^^^^^^^^^^
| |
| not allowed in struct literals
| help: replace it with an appropriate type: `/*Type*/`

error: aborting due to 2 previous errors

Loading