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

Invalid span when emitting a diagnostic note on the top-level module #63091

Closed
jakubadamw opened this issue Jul 28, 2019 · 3 comments
Closed
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@jakubadamw
Copy link
Contributor

jakubadamw commented Jul 28, 2019

While I was investigating a fix for #63064, I noticed one problematic error case, where, as it turns out, the compiler emits a broken span.

fn main() {}

trait Foo<X = Box<dyn Foo>> {}

This is the error I'm getting with the nightly:

error[E0391]: cycle detected when processing `Foo::X`
 --> ./src/test/ui/cycle-trait/cycle-trait-default-type-trait-oneliner.rs:3:23
  |
3 | trait Foo<X = Box<dyn Foo>> {}
  |                       ^^^
  |
  = note: ...which again requires processing `Foo::X`, completing the cycle
note: cycle used when collecting item types in top-level module
 --> ./src/test/ui/cycle-trait/cycle-trait-default-type-trait-oneliner.rs:1:1
  |
1 | fn main() {}
  | ^^^^^^^^^

The second span should extend to the whole top-level module but instead terminates at the first {. This is due to how SourceMap::def_span() is defined:

pub fn def_span(&self, sp: Span) -> Span {
self.span_until_char(sp, '{')
}

This works kind of okay for all item definitions, except for top-level modules. I did try to come up with a way to fix it up in libsyntax, but there's no clean way of doing it as it only deals with Spans. I figured the place to look at would be:

if let Some((span, query)) = usage {
err.span_note(fix_span(span, &query),
&format!("cycle used when {}", query.describe(self)));

but I am not at all familiar with the new query system, so decided to file this issue.

@jakubadamw
Copy link
Contributor Author

jakubadamw commented Jul 28, 2019

@eddyb, there's a relevant-looking FIXME with your name in that file:

// FIXME(eddyb) Get more valid Span's on queries.
pub fn default_span(&self, tcx: TyCtxt<$tcx>, span: Span) -> Span {
if !span.is_dummy() {
return span;
}
// The def_span query is used to calculate default_span,
// so exit to avoid infinite recursion
if let Query::def_span(..) = *self {
return span
}

Would you have an idea on a good fix for this? Seemed like an easy one for me at first but then I noticed that it would involve meddling with this plumbing.

@jakubadamw jakubadamw changed the title Invalid span when emitting a diagnostic notice on the top-level module Invalid span when emitting a diagnostic note on the top-level module Jul 28, 2019
@jonas-schievink jonas-schievink added A-diagnostics Area: Messages for errors, warnings, and lints C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 28, 2019
@eddyb
Copy link
Member

eddyb commented Aug 9, 2019

That def_span is pretty weird, cc @petrochenkov

@estebank
Copy link
Contributor

Current output:

error[[E0391]](https://doc.rust-lang.org/stable/error_codes/E0391.html): cycle detected when computing type of `Foo::X`
 --> src/main.rs:3:23
  |
3 | trait Foo<X = Box<dyn Foo>> {}
  |                       ^^^
  |
  = note: ...which immediately requires computing type of `Foo::X` again
note: cycle used when collecting item types in top-level module
 --> src/main.rs:1:1
  |
1 | / fn main() {}
2 | |
3 | | trait Foo<X = Box<dyn Foo>> {}
  | |______________________________^

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants