Skip to content

Commit 9df116c

Browse files
authored
Rollup merge of rust-lang#134192 - nnethercote:rm-Lexer-Parser-dep, r=compiler-errors
Remove `Lexer`'s dependency on `Parser`. Lexing precedes parsing, as you'd expect: `Lexer` creates a `TokenStream` and `Parser` then parses that `TokenStream`. But, in a horrendous violation of layering abstractions and common sense, `Lexer` depends on `Parser`! The `Lexer::unclosed_delim_err` method does some error recovery that relies on creating a `Parser` to do some post-processing of the `TokenStream` that the `Lexer` just created. This commit just removes `unclosed_delim_err`. This change removes `Lexer`'s dependency on `Parser`, and also means that `lex_token_tree`'s return value can have a more typical form. The cost is slightly worse error messages in two obscure cases, as shown in these tests: - tests/ui/parser/brace-in-let-chain.rs: there is slightly less explanation in this case involving an extra `{`. - tests/ui/parser/diff-markers/unclosed-delims{,-in-macro}.rs: the diff marker detection is no longer supported (because that detection is implemented in the parser). In my opinion this cost is outweighed by the magnitude of the code cleanup. r? `@chenyukang`
2 parents a8ec69b + 2e412fe commit 9df116c

File tree

8 files changed

+68
-174
lines changed

8 files changed

+68
-174
lines changed

Diff for: compiler/rustc_parse/src/lexer/mod.rs

+23-17
Original file line numberDiff line numberDiff line change
@@ -69,24 +69,30 @@ pub(crate) fn lex_token_trees<'psess, 'src>(
6969
token: Token::dummy(),
7070
diag_info: TokenTreeDiagInfo::default(),
7171
};
72-
let (_open_spacing, stream, res) = lexer.lex_token_trees(/* is_delimited */ false);
73-
let unmatched_delims = lexer.diag_info.unmatched_delims;
74-
75-
if res.is_ok() && unmatched_delims.is_empty() {
76-
Ok(stream)
77-
} else {
78-
// Return error if there are unmatched delimiters or unclosed delimiters.
79-
// We emit delimiter mismatch errors first, then emit the unclosing delimiter mismatch
80-
// because the delimiter mismatch is more likely to be the root cause of error
81-
let mut buffer: Vec<_> = unmatched_delims
82-
.into_iter()
83-
.filter_map(|unmatched_delim| make_unclosed_delims_error(unmatched_delim, psess))
84-
.collect();
85-
if let Err(errs) = res {
86-
// Add unclosing delimiter or diff marker errors
87-
buffer.extend(errs);
72+
let res = lexer.lex_token_trees(/* is_delimited */ false);
73+
74+
let mut unmatched_delims: Vec<_> = lexer
75+
.diag_info
76+
.unmatched_delims
77+
.into_iter()
78+
.filter_map(|unmatched_delim| make_unclosed_delims_error(unmatched_delim, psess))
79+
.collect();
80+
81+
match res {
82+
Ok((_open_spacing, stream)) => {
83+
if unmatched_delims.is_empty() {
84+
Ok(stream)
85+
} else {
86+
// Return error if there are unmatched delimiters or unclosed delimiters.
87+
Err(unmatched_delims)
88+
}
89+
}
90+
Err(errs) => {
91+
// We emit delimiter mismatch errors first, then emit the unclosing delimiter mismatch
92+
// because the delimiter mismatch is more likely to be the root cause of error
93+
unmatched_delims.extend(errs);
94+
Err(unmatched_delims)
8895
}
89-
Err(buffer)
9096
}
9197
}
9298

Diff for: compiler/rustc_parse/src/lexer/tokentrees.rs

+14-80
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,18 @@
11
use rustc_ast::token::{self, Delimiter, Token};
22
use rustc_ast::tokenstream::{DelimSpacing, DelimSpan, Spacing, TokenStream, TokenTree};
33
use rustc_ast_pretty::pprust::token_to_string;
4-
use rustc_errors::{Applicability, Diag};
5-
use rustc_span::symbol::kw;
4+
use rustc_errors::Diag;
65

76
use super::diagnostics::{report_suspicious_mismatch_block, same_indentation_level};
87
use super::{Lexer, UnmatchedDelim};
9-
use crate::Parser;
108

119
impl<'psess, 'src> Lexer<'psess, 'src> {
1210
// Lex into a token stream. The `Spacing` in the result is that of the
1311
// opening delimiter.
1412
pub(super) fn lex_token_trees(
1513
&mut self,
1614
is_delimited: bool,
17-
) -> (Spacing, TokenStream, Result<(), Vec<Diag<'psess>>>) {
15+
) -> Result<(Spacing, TokenStream), Vec<Diag<'psess>>> {
1816
// Move past the opening delimiter.
1917
let open_spacing = self.bump_minimal();
2018

@@ -27,25 +25,25 @@ impl<'psess, 'src> Lexer<'psess, 'src> {
2725
debug_assert!(!matches!(delim, Delimiter::Invisible(_)));
2826
buf.push(match self.lex_token_tree_open_delim(delim) {
2927
Ok(val) => val,
30-
Err(errs) => return (open_spacing, TokenStream::new(buf), Err(errs)),
28+
Err(errs) => return Err(errs),
3129
})
3230
}
3331
token::CloseDelim(delim) => {
3432
// Invisible delimiters cannot occur here because `TokenTreesReader` parses
3533
// code directly from strings, with no macro expansion involved.
3634
debug_assert!(!matches!(delim, Delimiter::Invisible(_)));
37-
return (
38-
open_spacing,
39-
TokenStream::new(buf),
40-
if is_delimited { Ok(()) } else { Err(vec![self.close_delim_err(delim)]) },
41-
);
35+
return if is_delimited {
36+
Ok((open_spacing, TokenStream::new(buf)))
37+
} else {
38+
Err(vec![self.close_delim_err(delim)])
39+
};
4240
}
4341
token::Eof => {
44-
return (
45-
open_spacing,
46-
TokenStream::new(buf),
47-
if is_delimited { Err(vec![self.eof_err()]) } else { Ok(()) },
48-
);
42+
return if is_delimited {
43+
Err(vec![self.eof_err()])
44+
} else {
45+
Ok((open_spacing, TokenStream::new(buf)))
46+
};
4947
}
5048
_ => {
5149
// Get the next normal token.
@@ -107,10 +105,7 @@ impl<'psess, 'src> Lexer<'psess, 'src> {
107105
// Lex the token trees within the delimiters.
108106
// We stop at any delimiter so we can try to recover if the user
109107
// uses an incorrect delimiter.
110-
let (open_spacing, tts, res) = self.lex_token_trees(/* is_delimited */ true);
111-
if let Err(errs) = res {
112-
return Err(self.unclosed_delim_err(tts, errs));
113-
}
108+
let (open_spacing, tts) = self.lex_token_trees(/* is_delimited */ true)?;
114109

115110
// Expand to cover the entire delimited token tree.
116111
let delim_span = DelimSpan::from_pair(pre_span, self.token.span);
@@ -247,67 +242,6 @@ impl<'psess, 'src> Lexer<'psess, 'src> {
247242
this_spacing
248243
}
249244

250-
fn unclosed_delim_err(
251-
&mut self,
252-
tts: TokenStream,
253-
mut errs: Vec<Diag<'psess>>,
254-
) -> Vec<Diag<'psess>> {
255-
// If there are unclosed delims, see if there are diff markers and if so, point them
256-
// out instead of complaining about the unclosed delims.
257-
let mut parser = Parser::new(self.psess, tts, None);
258-
let mut diff_errs = vec![];
259-
// Suggest removing a `{` we think appears in an `if`/`while` condition.
260-
// We want to suggest removing a `{` only if we think we're in an `if`/`while` condition,
261-
// but we have no way of tracking this in the lexer itself, so we piggyback on the parser.
262-
let mut in_cond = false;
263-
while parser.token != token::Eof {
264-
if let Err(diff_err) = parser.err_vcs_conflict_marker() {
265-
diff_errs.push(diff_err);
266-
} else if parser.is_keyword_ahead(0, &[kw::If, kw::While]) {
267-
in_cond = true;
268-
} else if matches!(
269-
parser.token.kind,
270-
token::CloseDelim(Delimiter::Brace) | token::FatArrow
271-
) {
272-
// End of the `if`/`while` body, or the end of a `match` guard.
273-
in_cond = false;
274-
} else if in_cond && parser.token == token::OpenDelim(Delimiter::Brace) {
275-
// Store the `&&` and `let` to use their spans later when creating the diagnostic
276-
let maybe_andand = parser.look_ahead(1, |t| t.clone());
277-
let maybe_let = parser.look_ahead(2, |t| t.clone());
278-
if maybe_andand == token::OpenDelim(Delimiter::Brace) {
279-
// This might be the beginning of the `if`/`while` body (i.e., the end of the
280-
// condition).
281-
in_cond = false;
282-
} else if maybe_andand == token::AndAnd && maybe_let.is_keyword(kw::Let) {
283-
let mut err = parser.dcx().struct_span_err(
284-
parser.token.span,
285-
"found a `{` in the middle of a let-chain",
286-
);
287-
err.span_suggestion(
288-
parser.token.span,
289-
"consider removing this brace to parse the `let` as part of the same chain",
290-
"",
291-
Applicability::MachineApplicable,
292-
);
293-
err.span_label(
294-
maybe_andand.span.to(maybe_let.span),
295-
"you might have meant to continue the let-chain here",
296-
);
297-
errs.push(err);
298-
}
299-
}
300-
parser.bump();
301-
}
302-
if !diff_errs.is_empty() {
303-
for err in errs {
304-
err.cancel();
305-
}
306-
return diff_errs;
307-
}
308-
errs
309-
}
310-
311245
fn close_delim_err(&mut self, delim: Delimiter) -> Diag<'psess> {
312246
// An unexpected closing delimiter (i.e., there is no matching opening delimiter).
313247
let token_str = token_to_string(&self.token);

Diff for: tests/ui/parser/brace-in-let-chain.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,15 @@
33
#![feature(let_chains)]
44
fn main() {
55
if let () = ()
6-
&& let () = () { //~ERROR: found a `{` in the middle of a let-chain
6+
&& let () = () {
77
&& let () = ()
88
{
99
}
1010
}
1111

1212
fn quux() {
1313
while let () = ()
14-
&& let () = () { //~ERROR: found a `{` in the middle of a let-chain
14+
&& let () = () {
1515
&& let () = ()
1616
{
1717
}

Diff for: tests/ui/parser/brace-in-let-chain.stderr

+1-29
Original file line numberDiff line numberDiff line change
@@ -27,33 +27,5 @@ LL | }
2727
LL | }
2828
| ^
2929

30-
error: found a `{` in the middle of a let-chain
31-
--> $DIR/brace-in-let-chain.rs:14:24
32-
|
33-
LL | && let () = () {
34-
| ^
35-
LL | && let () = ()
36-
| ------ you might have meant to continue the let-chain here
37-
|
38-
help: consider removing this brace to parse the `let` as part of the same chain
39-
|
40-
LL - && let () = () {
41-
LL + && let () = ()
42-
|
43-
44-
error: found a `{` in the middle of a let-chain
45-
--> $DIR/brace-in-let-chain.rs:6:24
46-
|
47-
LL | && let () = () {
48-
| ^
49-
LL | && let () = ()
50-
| ------ you might have meant to continue the let-chain here
51-
|
52-
help: consider removing this brace to parse the `let` as part of the same chain
53-
|
54-
LL - && let () = () {
55-
LL + && let () = ()
56-
|
57-
58-
error: aborting due to 3 previous errors
30+
error: aborting due to 1 previous error
5931

+4-2
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
1+
// The diff marker detection was removed for this example, because it relied on
2+
// the lexer having a dependency on the parser, which was horrible.
3+
14
macro_rules! foo {
25
<<<<<<< HEAD
3-
//~^ ERROR encountered diff marker
46
() {
57
=======
68
() { //
79
>>>>>>> 7a4f13c blah blah blah
810
}
9-
}
11+
} //~ this file contains an unclosed delimiter
+10-17
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,16 @@
1-
error: encountered diff marker
2-
--> $DIR/unclosed-delims-in-macro.rs:2:1
1+
error: this file contains an unclosed delimiter
2+
--> $DIR/unclosed-delims-in-macro.rs:11:48
33
|
4+
LL | macro_rules! foo {
5+
| - unclosed delimiter
46
LL | <<<<<<< HEAD
5-
| ^^^^^^^ between this marker and `=======` is the code that we're merging into
7+
LL | () {
8+
| - this delimiter might not be properly closed...
69
...
7-
LL | =======
8-
| ------- between this marker and `>>>>>>>` is the incoming code
9-
LL | () { //
10-
LL | >>>>>>> 7a4f13c blah blah blah
11-
| ^^^^^^^ this marker concludes the conflict region
12-
|
13-
= note: conflict markers indicate that a merge was started but could not be completed due to merge conflicts
14-
to resolve a conflict, keep only the code you want and then delete the lines containing conflict markers
15-
= help: if you're having merge conflicts after pulling new code:
16-
the top section is the code you already had and the bottom section is the remote code
17-
if you're in the middle of a rebase:
18-
the top section is the code being rebased onto and the bottom section is the code coming from the current commit being rebased
19-
= note: for an explanation on these markers from the `git` documentation:
20-
visit <https://git-scm.com/book/en/v2/Git-Tools-Advanced-Merging#_checking_out_conflicts>
10+
LL | }
11+
| - ^
12+
| |
13+
| ...as it matches this but it has different indentation
2114

2215
error: aborting due to 1 previous error
2316

Diff for: tests/ui/parser/diff-markers/unclosed-delims.rs

+4-10
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,12 @@
1+
// The diff marker detection was removed for this example, because it relied on
2+
// the lexer having a dependency on the parser, which was horrible.
3+
14
mod tests {
25
#[test]
36
<<<<<<< HEAD
4-
//~^ ERROR encountered diff marker
5-
//~| NOTE between this marker and `=======`
6-
7-
//~| NOTE conflict markers indicate that
8-
//~| HELP if you're having merge conflicts
9-
//~| NOTE for an explanation on these markers
10-
117
fn test1() {
128
=======
13-
//~^ NOTE between this marker and `>>>>>>>`
149
fn test2() {
1510
>>>>>>> 7a4f13c blah blah blah
16-
//~^ NOTE this marker concludes the conflict region
1711
}
18-
}
12+
} //~ this file contains an unclosed delimiter

Diff for: tests/ui/parser/diff-markers/unclosed-delims.stderr

+10-17
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,16 @@
1-
error: encountered diff marker
2-
--> $DIR/unclosed-delims.rs:3:1
1+
error: this file contains an unclosed delimiter
2+
--> $DIR/unclosed-delims.rs:12:48
33
|
4-
LL | <<<<<<< HEAD
5-
| ^^^^^^^ between this marker and `=======` is the code that we're merging into
4+
LL | mod tests {
5+
| - unclosed delimiter
66
...
7-
LL | =======
8-
| ------- between this marker and `>>>>>>>` is the incoming code
7+
LL | fn test1() {
8+
| - this delimiter might not be properly closed...
99
...
10-
LL | >>>>>>> 7a4f13c blah blah blah
11-
| ^^^^^^^ this marker concludes the conflict region
12-
|
13-
= note: conflict markers indicate that a merge was started but could not be completed due to merge conflicts
14-
to resolve a conflict, keep only the code you want and then delete the lines containing conflict markers
15-
= help: if you're having merge conflicts after pulling new code:
16-
the top section is the code you already had and the bottom section is the remote code
17-
if you're in the middle of a rebase:
18-
the top section is the code being rebased onto and the bottom section is the code coming from the current commit being rebased
19-
= note: for an explanation on these markers from the `git` documentation:
20-
visit <https://git-scm.com/book/en/v2/Git-Tools-Advanced-Merging#_checking_out_conflicts>
10+
LL | }
11+
| - ^
12+
| |
13+
| ...as it matches this but it has different indentation
2114

2215
error: aborting due to 1 previous error
2316

0 commit comments

Comments
 (0)