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

Fix E0132 error display #35477

Merged
merged 1 commit into from
Aug 10, 2016
Merged

Fix E0132 error display #35477

merged 1 commit into from
Aug 10, 2016

Conversation

GuillaumeGomez
Copy link
Member

Error and note now use the same span.

r? @jonathandturner

@@ -261,9 +261,10 @@ fn check_start_fn_ty(ccx: &CrateCtxt,
match it.node {
hir::ItemFn(_,_,_,_,ref ps,_)
if ps.is_parameterized() => {
struct_span_err!(tcx.sess, start_span, E0132,
let sp = ps.span().unwrap();
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks okay, though I'm not 100% sure there's always a span if it's .is_parameterized().

We could support a fallback with something like:

let sp = if let Some(span) = ps.span() { span } else { start_span };

@sophiajt
Copy link
Contributor

sophiajt commented Aug 7, 2016

Looks okay, though I left a comment on one way we might allow it to fallback to the larger span if there's ever a case we don't have the smaller span.

@GuillaumeGomez
Copy link
Member Author

Updated.

@sophiajt
Copy link
Contributor

sophiajt commented Aug 7, 2016

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Aug 7, 2016

📌 Commit e40df1c has been approved by jonathandturner

@bors
Copy link
Contributor

bors commented Aug 8, 2016

⌛ Testing commit e40df1c with merge b605013...

@bors
Copy link
Contributor

bors commented Aug 8, 2016

💔 Test failed - auto-win-gnu-64-opt

@GuillaumeGomez
Copy link
Member Author

That is quite the strange error!

@sophiajt
Copy link
Contributor

sophiajt commented Aug 8, 2016

@bors retry

@alexcrichton
Copy link
Member

@bors: retry

  • not sure why...

sophiajt pushed a commit to sophiajt/rust that referenced this pull request Aug 8, 2016
…ndturner

Fix E0132 error display

Error and note now use the same span.

r? @jonathandturner
steveklabnik added a commit to steveklabnik/rust that referenced this pull request Aug 10, 2016
…ndturner

Fix E0132 error display

Error and note now use the same span.

r? @jonathandturner
bors added a commit that referenced this pull request Aug 10, 2016
@bors bors merged commit e40df1c into rust-lang:master Aug 10, 2016
@GuillaumeGomez GuillaumeGomez deleted the fix_E0132 branch August 15, 2016 10:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants