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

Simplify librustc_errors #34789

Merged
merged 19 commits into from
Jul 17, 2016
Merged

Simplify librustc_errors #34789

merged 19 commits into from
Jul 17, 2016

Conversation

sophiajt
Copy link
Contributor

@sophiajt sophiajt commented Jul 12, 2016

This is part 2 of the error crate refactor, starting with #34403.

In this refactor, I focused on slimming down the error crate to fewer moving parts. As such, I've removed quite a few parts and replaced the with simpler, straight-line code. Specifically, this PR:

  • Removes BasicEmitter
  • Remove emit from emitter, leaving emit_struct
  • Renames emit_struct to emit
  • Removes CoreEmitter and focuses on a single Emitter
  • Implements the latest changes to error format RFC (Relinquish the alignof keyword in rustrt #1644)
  • Removes (now-unused) code in emitter.rs and snippet.rs
  • Moves more tests to the UI tester, removing some duplicate tests in the process

There is probably more that could be done with some additional refactoring, but this felt like it was getting to a good state.

r? @alexcrichton cc: @Manishearth (as there may be breaking changes in stuff I removed/changed)

}
impl Emitter for EmitterWriter {
fn emit(&mut self, db: &DiagnosticBuilder) {
let old_school = match self.format_mode {
Copy link
Member

Choose a reason for hiding this comment

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

I was massively confused about "school" here vs "skool", please udpate.

Copy link
Member

Choose a reason for hiding this comment

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

To be clear, I think "skool" is the correct spelling

Copy link
Contributor Author

Choose a reason for hiding this comment

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

-_-

@alexcrichton
Copy link
Member

Looks good to me! I didn't look too closely at the code changes as it looked like it was mostly just stuff moving around, but I like the simplifying refactors here quite a bit!

r? @Manishearth -- if this plays into a syntax breaking change I'll defer to you, otherwise this looks ready to r+ to me

@Manishearth
Copy link
Member

Not really a libsyntax breaking change, but might affect syntex -- cc @dtolnay

@sophiajt
Copy link
Contributor Author

Looks like we got the okay from @Manishearth and @dtolnay.

@bors r=alexcrichton

@bors
Copy link
Contributor

bors commented Jul 15, 2016

📌 Commit c7158a1 has been approved by alexcrichton

@bors
Copy link
Contributor

bors commented Jul 16, 2016

⌛ Testing commit c7158a1 with merge c7e060a...

@bors
Copy link
Contributor

bors commented Jul 16, 2016

💔 Test failed - auto-win-msvc-32-opt

@alexcrichton
Copy link
Member

@bors
Copy link
Contributor

bors commented Jul 17, 2016

⌛ Testing commit c7158a1 with merge 7ed6068...

bors added a commit that referenced this pull request Jul 17, 2016
Simplify librustc_errors

This is part 2 of the error crate refactor, starting with #34403.

In this refactor, I focused on slimming down the error crate to fewer moving parts.  As such, I've removed quite a few parts and replaced the with simpler, straight-line code.  Specifically, this PR:

* Removes BasicEmitter
* Remove emit from emitter, leaving emit_struct
* Renames emit_struct to emit
* Removes CoreEmitter and focuses on a single Emitter
* Implements the latest changes to error format RFC (#1644)
* Removes (now-unused) code in emitter.rs and snippet.rs
* Moves more tests to the UI tester, removing some duplicate tests in the process

There is probably more that could be done with some additional refactoring, but this felt like it was getting to a good state.

r? @alexcrichton   cc: @Manishearth (as there may be breaking changes in stuff I removed/changed)
@bors bors merged commit c7158a1 into rust-lang:master Jul 17, 2016
@sophiajt sophiajt deleted the simplify_liberror branch July 18, 2016 13:15
@kennytm kennytm mentioned this pull request May 1, 2017
bors added a commit that referenced this pull request May 7, 2017
Fix issue #41652

Fix issue #41652. Don't print anything in `render_source_line()` if no source code is given.

(cc @jonathandturner #34789)
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