Add primary label to incorrect doc comments

[estebank]

Desired output:

error: expected item after doc comment
  --> $DIR/doc-before-extern-rbrace.rs:2:5
   |
LL |     /// hi
   |     ^^^^^^ this doc comment doesn't document anything

[JohnTitor]

@estebank I came up with this change (src/libsyntax/parse/parser.rs:4478), but this may be wrong. What do you think?

self.struct_span_err(self.prev_span, message)
    .span_label(self.prev_span, "this doc comment doesn't document anything")
    .emit();

[estebank]

@JohnTitor that seems correct to me. I believe there are a couple of other places where we check for incorrect doc comments that will also need to have the span_label added.

[JohnTitor]

@estebank Okay. This is stderr after changing.

error: expected item after doc comment
  --> $DIR/doc-before-extern-rbrace.rs:2:5
   |
LL |     /// hi
   |     ^^^^^^ this doc comment doesn't document anything

error: expected one of `crate`, `fn`, `pub`, `static`, or `type`, found `}`
  --> $DIR/doc-before-extern-rbrace.rs:4:1
   |
LL |     /// hi
   |           - expected one of `crate`, `fn`, `pub`, `static`, or `type` here
LL |     //~^ ERROR expected item after doc comment
LL | }
   | ^ unexpected token

error: aborting due to 2 previous errors

This may be unfavorable since first error doesn't abort processing.

[estebank]

That's less than ideal. Could you show the diff from your change? It would be unusual the change from span_err to struct_span_err to cause that difference in output.

[JohnTitor]

here JohnTitor@9ab2320
This change isn't good because doc comments and attributes don't separate. But it wouldn't be matter.

[JohnTitor]

@estebank I found the cause, so I can add spans. But I cannot find other place to add spans, could you tell me?

[estebank]

@JohnTitor I'm not sure I understand the problem, could you expand?

[JohnTitor]

@estebank Sure.
struct_span_err outputs an error using DiagnosticBuilder, but span_err does using emit.
emit checks if continuing after error. This abort_if_errors returns FatalError.raise().

rust/src/librustc_errors/lib.rs

Lines 646 to 651 in e2f221c

	pub fn abort_if_errors(&self) {
	if self.err_count() == 0 {
	return;
	}
	FatalError.raise();
	}

When I added FatalError.raise() after emit() in src/libsyntax/parse/parser.rs, there is one error(aborted process).

[JohnTitor]

@estebank Is my solution good, or is there better solution?
If it’s not too much trouble, I'd like to make a PR and ask you to review.

[estebank]

@JohnTitor I would like to avoid having a FatalError.raise() after this. I noticed that the current method expected_item_err doesn't return anything, it should return a PResult with the DiangosticBuilder created, and be called with a ? now. That way when we encounter this error we no longer attempt to parse the item expected after, ending up again with only one error. Other than that, I think you're in the right track.

[JohnTitor]

@estebank Is this related #52790 ?

[estebank]

@JohnTitor it is related. Given that you'd start returning the DiagnosticBuilder up the call chain, you could attach a label pointing at the following token's span fairly easily.