Make diff conflict marker error higher priority than unclosed delimeter

[joshka]

I tried this code:

mod tests {
    #[test]
<<<<<<< HEAD
    fn test1() {
=======
    fn test2() {
>>>>>>> 7a4f13c blah blah blah
    }
}

I expected to see this happen: explanation
error: encountered diff marker

Instead, this happened: explanation
error: this file contains an unclosed delimiter

Meta

rustc --version --verbose:

rustc 1.72.1 (d5c2e9c34 2023-09-13)
binary: rustc
commit-hash: d5c2e9c342b358556da91d61ed4133f6f50fc0c3
commit-date: 2023-09-13
host: aarch64-apple-darwin
release: 1.72.1
LLVM version: 16.0.5

[Rageking8]

@rustbot label +T-compiler +A-diagnostics

[fmease]

Right, this is happens because the unclosed delimiter error is emitted in the lexer (rustc_parse::lexer not rustc_lexer) before the parser (which is responsible for detecting diff markers) gets the chance to parse anything.

[joshka]

Right, this is happens because the unclosed delimiter error is emitted in the lexer (rustc_parse::lexer not rustc_lexer) before the parser (which is responsible for detecting diff markers) gets the chance to parse anything.

Reading between the line that sounds like it fits in the difficult to implement with only a small amount of value basket. Does this sound right?

[workingjubilee]

correct, you would have to be able to traverse the previous diagnostic errors already to-be-emitted and then swallow the previous one, I think. I don't know if our diagnostic architecture even allows that?

[estebank]

We have done similar things by storing some related unemitted DiagnosticBuilders and ferry them around from one stage to the next and only emit them try to find them later through some key (like a Span) and if present we remove it/delay_as_bug, but this is very ad-hoc at the moment.

[fmease]

So this regressed in #108297 which made unclosed delimiters a hard error. Before, the diff conflict marker error was emitted just fine:

stderr

error: this file contains an unclosed delimiter
 --> diff_marker.rs:9:3
  |
1 | mod tests {
  |           - unclosed delimiter
...
4 |     fn test1() {
  |                - this delimiter might not be properly closed...
...
9 | }
  | - ^
  | |
  | ...as it matches this but it has different indentation

error: encountered diff marker
 --> diff_marker.rs:3:1
  |
3 | <<<<<<< HEAD
  | ^^^^^^^ after this is the code before the merge
4 |     fn test1() {
5 | =======
  | -------
6 |     fn test2() {
7 | >>>>>>> 7a4f13c blah blah blah
  | ^^^^^^^ above this are the incoming code changes
  |
  = help: if you're having merge conflicts after pulling new code, the top section is the code you already had and the bottom section is the remote code
  = help: if you're in the middle of a rebase, the top section is the code being rebased onto and the bottom section is the code coming from the current commit being rebased
  = note: for an explanation on these markers from the `git` documentation, visit <https://git-scm.com/book/en/v2/Git-Tools-Advanced-Merging#_checking_out_conflicts>

error: aborting due to 2 previous errors

Without being an expert in this field, this should in theory be fixable by moving the conflict marker detection to the tokentree parser in rustc_parse::lexer, right? This would also greatly simplify the rust parser which at the moment is sprinkled with checks at specific places only. I might not see the full picture though. I faintly remember something similar being discussed when estebank implemented the current approach. To me that sounds better than using stashed diagnostics for this (which would need to be stashed in the tokentree parser anyway?).