Move errors from libsyntax into their own crate #34403

sophiajt · 2016-06-21T22:20:28Z

This PR refactors the 'errors' part of libsyntax into its own crate (librustc_errors). This is the first part of a few refactorings to simplify error reporting and potentially support more output formats (like a standardized JSON output and possibly an --explain mode that can work with the user's code), though this PR stands on its own and doesn't assume further changes.

As part of separating out the errors crate, I have also refactored the code position portion of codemap into its own crate (libsyntax_pos). While it's helpful to have the common code positions in a separate crate for the new errors crate, this may also enable further simplifications in the future.

r? @alexcrichton

nagisa · 2016-06-21T22:59:18Z

potentially plugin-[breaking-change]

alexcrichton · 2016-06-21T23:02:21Z

src/libsyntax_pos/lib.rs

+//!
+//! This API is completely unstable and subject to change.
+
+#![crate_name = "syntax_pos"]


I'm not sure if it's too late for this, but maybe this crate should be called rustc_codemap to follow the trend of rustc_* and it's basically where the module came from as well.

Me and @alexcrichton chatted offline a bit about this. A rustc_* name may make sense but we couldn't think of a good fit. Will probably move that way in the future, but the name seems good enough for now.

alexcrichton · 2016-06-21T23:06:54Z

Looks good to me! cc @Manishearth, is there a protocol for negotiating landing these kinds of changes nowadays?

sophiajt · 2016-06-21T23:59:33Z

thanks @alexcrichton - updated with your suggestions (minus the name one which we chatted about offline).

Manishearth · 2016-06-22T05:36:43Z

(travis fails)

@alexcrichton yes! you just say @manishearth: r+ or something like that, cc #31645, and then wait for a breaking batch. If the PR is bitrot-susceptible or is something otherwise urgent, let me know and I will make a batch, whilst encouraging others to squeeze their own breaking changes in. Fortunately, such a batch is already in progress right now (waiting on a fixup/review of #34316), so this should just fit in.

Manishearth · 2016-06-22T05:37:36Z

@erickt @dtolnay fyi this looks like a major bit of breakage for you. I will let the breaking batch PR simmer for a day (once it exists) before trying to merge it so that you get ample time to get this to work properly. Let me know if you need more time.

erickt · 2016-06-22T13:30:24Z

@Manishearth: yikes, this is definitely going to cause some syntex fallout. Thanks for the heads up.

sophiajt · 2016-06-22T17:09:31Z

@Manishearth - thanks! This PR touches a fair number of files, so I'm all for batching it up with the other breaking changes. With luck I'll have fixed the last of the travis issues...

alexcrichton · 2016-06-22T22:07:41Z

@Manishearth yeah this is good to go whenever at this point (r+ from me)

@jonathandturner looks like the travis failure may be legit?

sophiajt · 2016-06-22T22:08:37Z

@alexcrichton - it's legit, chasing down the last(?) of the errors as we speak

GuillaumeGomez · 2016-06-22T22:47:08Z

Once merged, I'll add missing error codes into libsyntax (FINALLY!). Very nice work! 👍

bors · 2016-06-23T11:27:46Z

☔ The latest upstream changes (presumably #34253) made this pull request unmergeable. Please resolve the merge conflicts.

jseyfried · 2016-06-24T20:20:10Z

@jonathandturner If we re-export syntax_pos in syntax::diagnostics and re-export rustc_errors in syntax::diagnostics, would this no longer by a syntax-[breaking-chage]?

I think it would be a good idea to re-export everything relevant to plugin authors in libsyntax anyway.

sophiajt · 2016-06-24T20:23:59Z

@jseyfried - Hmm I think the idea is more "refactor so that we can make deeper changes". This has the unfortunate side effect that it may break plugin authors from time to time. The current errors system has layers of different styles and approaches, but I'd like to get it simplified to one style that we can use and then output multiple types of output.

jseyfried · 2016-06-24T20:32:17Z

@jonathandturner
Agreed that a simpler error system would be excellent, even if it breaks plugin authors.

Would we ever not be able to re-export everything relevant to plugin authors in libsyntax? I think re-exporting everything in libsyntax would be a good idea regardless of how the error system ends up being structured, if this is possible.

Also, perhaps we could add the re-exports for plugin authors in the PR but not use them in rustc itself? We could always remove the re-exports when we make deeper changes that would cause breakage for plugin authors anyway.

sophiajt · 2016-06-24T20:41:08Z

@jseyfried - yeah, I can definitely do that. I think you're right that this would prevent immediate breakage. We can still give plugin authors a heads up that the further simplifications might break them down the road...

jseyfried · 2016-06-24T20:47:13Z

Hopefully they know that -- we try to minimize and batch up plugin-[breaking-changes] but we still make them fairly often (c.f. #31645).

sophiajt · 2016-06-25T01:14:07Z

Woot! Pinging @Manishearth - looks like we've passed travis and cut down the number of breaking changes for this patch.

jseyfried · 2016-06-25T01:16:24Z

@jonathandturner Nice!

cut down the number of breaking changes

Are there any syntax-[breaking-changes] left? If so, what are they?

sophiajt · 2016-06-25T01:27:56Z

@jseyfried - Ah good point. In theory, this should be source compatible with what we had before, though I'm not 100% sure (a fair bit got moved around). Still might be worth batching with breaking changes...

jseyfried · 2016-06-25T02:03:22Z

@Manishearth what do you think?

Manishearth · 2016-06-25T03:15:44Z

We still have the batch going on; @jseyfried want to finish it? Make a PR that includes this and send it to try. We can merge it tomorrow.

jseyfried · 2016-06-25T03:41:51Z

@Manishearth I'll finish up #34424 (and include #34436) in a couple of hours.

…excrichton This PR refactors the 'errors' part of libsyntax into its own crate (librustc_errors). This is the first part of a few refactorings to simplify error reporting and potentially support more output formats (like a standardized JSON output and possibly an --explain mode that can work with the user's code), though this PR stands on its own and doesn't assume further changes. As part of separating out the errors crate, I have also refactored the code position portion of codemap into its own crate (libsyntax_pos). While it's helpful to have the common code positions in a separate crate for the new errors crate, this may also enable further simplifications in the future.

Batch up libsyntax breaking changes Batch of the following syntax-[breaking-change] changes: - #34213: Add a variant `Macro` to `TraitItemKind` - #34368: Merge the variant `QPath` of `PatKind` into the variant `PatKind::Path` - #34385: Move `syntax::ast::TokenTree` into a new module `syntax::tokenstream` - #33943: - Remove the type parameter from `visit::Visitor` - Remove `attr::WithAttrs` -- use `attr::HasAttrs` instead. - Change `fold_tt`/`fold_tts` to take token trees by value and avoid wrapping token trees in `Rc`. - Remove the field `ctxt` of `ast::Mac_` - Remove inherent method `attrs()` of types -- use the method `attrs` of `HasAttrs` instead. - #34316: - Remove `ast::Decl`/`ast::DeclKind` and add variants `Local` and `Item` to `StmtKind`. - Move the node id for statements from the `StmtKind` variants to a field of `Stmt` (making `Stmt` a struct instead of an alias for `Spanned<StmtKind>`) - Rename `ast::ExprKind::Again` to `Continue`. - #34339: Generalize and abstract `ThinAttributes` to `ThinVec<Attribute>` - Use `.into()` in convert between `Vec<Attribute>` and `ThinVec<Attribute>` - Use autoderef instead of `.as_attr_slice()` - #34436: Remove the optional expression from `ast::Block` and instead use a `StmtKind::Expr` at the end of the statement list. - #34403: Move errors into a separate crate (unlikely to cause breakage)

bors · 2016-06-28T04:47:25Z

☔ The latest upstream changes (presumably #34424) made this pull request unmergeable. Please resolve the merge conflicts.

jseyfried · 2016-06-28T05:45:13Z

@jonathandturner I believe this didn't get automatically marked as merged because of the above commit you added.

GuillaumeGomez · 2016-06-28T12:35:27Z

So I can start my part of the work! :D

@alexcrichton

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)

rust-highfive assigned alexcrichton Jun 21, 2016

alexcrichton reviewed Jun 21, 2016
View reviewed changes

Manishearth mentioned this pull request Jun 22, 2016

Tracking issue for libsyntax breaking changes #31645

Closed

sophiajt force-pushed the move_liberror branch from 5c50c17 to 37c78dc Compare June 22, 2016 17:07

jseyfried mentioned this pull request Jun 23, 2016

Batch up libsyntax breaking changes #34424

Merged

Jonathan Turner added 6 commits June 23, 2016 08:07

Move errors from libsyntax to its own crate

6ae3502

Address comments and fix travis warning

2829fbc

Address more travis errors

51deb4f

Consolidate codemap tests and fix more errors for travis

f2fe204

Fix touchy test to work with old and new error format

bad4869

Actually consolidate the CodeMap tests

b68e079

sophiajt force-pushed the move_liberror branch from f7f7adb to b68e079 Compare June 23, 2016 13:06

Jonathan Turner added 4 commits June 23, 2016 10:50

Move test helper functions to consolidated codemap testing

2b8bab0

Add missing refernce to RenderedLine

d4e79de

make old school mode a bit more configurable

80f1c78

Updating test I missed for old school refactor

bdd3f8f

Fix codemap tests to not double import

9f2a507

reexport errors from syntax. fix failing cfail test

bc14006

Fix rustbuild error building libsyntax

53fbc92

sophiajt closed this Jun 28, 2016

sophiajt deleted the move_liberror branch June 28, 2016 10:31

GuillaumeGomez mentioned this pull request Jun 28, 2016

Add error codes into libsyntax #34526

Closed

sophiajt mentioned this pull request Jul 12, 2016

Simplify librustc_errors #34789

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move errors from libsyntax into their own crate #34403

Move errors from libsyntax into their own crate #34403

sophiajt commented Jun 21, 2016

nagisa commented Jun 21, 2016

alexcrichton Jun 21, 2016

sophiajt Jun 21, 2016

alexcrichton commented Jun 21, 2016

sophiajt commented Jun 21, 2016

Manishearth commented Jun 22, 2016

Manishearth commented Jun 22, 2016

erickt commented Jun 22, 2016

sophiajt commented Jun 22, 2016

alexcrichton commented Jun 22, 2016

sophiajt commented Jun 22, 2016

GuillaumeGomez commented Jun 22, 2016

bors commented Jun 23, 2016

jseyfried commented Jun 24, 2016

sophiajt commented Jun 24, 2016

jseyfried commented Jun 24, 2016

sophiajt commented Jun 24, 2016

jseyfried commented Jun 24, 2016

sophiajt commented Jun 25, 2016

jseyfried commented Jun 25, 2016

sophiajt commented Jun 25, 2016

jseyfried commented Jun 25, 2016

Manishearth commented Jun 25, 2016

jseyfried commented Jun 25, 2016

bors commented Jun 28, 2016

jseyfried commented Jun 28, 2016

GuillaumeGomez commented Jun 28, 2016

Move errors from libsyntax into their own crate #34403

Move errors from libsyntax into their own crate #34403

Conversation

sophiajt commented Jun 21, 2016

nagisa commented Jun 21, 2016

alexcrichton Jun 21, 2016

Choose a reason for hiding this comment

sophiajt Jun 21, 2016

Choose a reason for hiding this comment

alexcrichton commented Jun 21, 2016

sophiajt commented Jun 21, 2016

Manishearth commented Jun 22, 2016

Manishearth commented Jun 22, 2016

erickt commented Jun 22, 2016

sophiajt commented Jun 22, 2016

alexcrichton commented Jun 22, 2016

sophiajt commented Jun 22, 2016

GuillaumeGomez commented Jun 22, 2016

bors commented Jun 23, 2016

jseyfried commented Jun 24, 2016

sophiajt commented Jun 24, 2016

jseyfried commented Jun 24, 2016

sophiajt commented Jun 24, 2016

jseyfried commented Jun 24, 2016

sophiajt commented Jun 25, 2016

jseyfried commented Jun 25, 2016

sophiajt commented Jun 25, 2016

jseyfried commented Jun 25, 2016

Manishearth commented Jun 25, 2016

jseyfried commented Jun 25, 2016

bors commented Jun 28, 2016

jseyfried commented Jun 28, 2016

GuillaumeGomez commented Jun 28, 2016