rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
97.53k stars 12.61k forks source link

`libsyntax/parse/parser.rs` became too big for GitHub #60015

Closed petrochenkov closed 5 years ago

petrochenkov commented 5 years ago

Very convenient "Blame" functionality doesn't work in particular.

The solution is to move something from that file. The obvious candidate for moving is diagnostics-related code, that can be moved into libsyntax/parse/diagnostics.rs.

Centril commented 5 years ago

After the diagnostics extraction, ...

...I think we should split the parser up into semantic categories "items", "expressions", "types", "patterns", and so on and so forth since that makes understanding the parser much easier. E.g. you could use some Parse trait which these each of these semantic categories implement.

Aside: rustc has some ridiculously large files; we should also seriously consider making tidy warn on too large files at minimum and eventually setting some hard boundaries (e.g. > 3000 LOC => build failure) to avoid files getting so large in the future. We'd need to lower the limit incrementally of course.

agnxy commented 5 years ago

@petrochenkov I'd like to work on this issue :smiley: I might need some guidance as I'm a beginner to Rust and syntax crate. I'm reading parser.rs to see what can be extracted to diagnostics.rs. Thanks.

petrochenkov commented 5 years ago

@agnxy Moving maybe_report_ambiguous_plus, maybe_recover_from_bad_type_plus, maybe_recover_from_bad_qpath, maybe_recover_from_bad_qpath_stage_2, maybe_consume_incorrect_semicolon, trait RecoverQPath and its impls would be a good start.

petrochenkov commented 5 years ago

In general, if any function does only error reporting or error recovery, it can be moved.

matklad commented 5 years ago

Heh, if I am not mistaking, parser.rs is the largest non-generated Rust file out there :) I use it for benchmarking my parsers and will be sad to see it go /s

On a more serious note, I quite like the grammar layout I use for rust-analyzer.

There's also a vague desire to make a parsing library, which will work inside rustc, syn and IDE. I don't think we should actively plan for this right now, but it might be a good idea to keep in mind. Specifically, it might be interesting to take a look at how swift's parser is organized: IIUC, it produces both the traditional AST, like rustc, and a Swift libsyntanx concrete syntax tree. This seems like a setup we might want to do as well, it is consistent with current RLS-2.0 approach.

agnxy commented 5 years ago

Hi,I have a noob question about the workflow. Since I may make more changes for this issue and #60348 is merged, should I create a new PR on a new branch? Thanks.

petrochenkov commented 5 years ago

@agnxy For already merged PRs the same branch can be reused, but it doesn't make any difference, there are no requirements on this or anything.

agnxy commented 5 years ago

Thanks @petrochenkov . I'll try to figure out more items for diagnostics.rs

agnxy commented 5 years ago

@petrochenkov , shall I also move recover_closing_delimiter, recover_seq_parse_error, report_invalid_macro_expansion_item, expected_item_err, err_dotdotdot_syntax and missing_assoc_item_kind_err to diagnostics.rs? I'm not sure if that's ok. Thanks a lot.

estebank commented 5 years ago

I believe this can be closed now. We should continue on the diet regardless.

ehuss commented 5 years ago

I still can't load https://github.com/rust-lang/rust/blame/master/src/libsyntax/parse/parser.rs

estebank commented 5 years ago

Weird, when I did it a week ago it loaded fine.

ehuss commented 5 years ago

It's a bit random. In the past, sometimes after getting the unicorn, hitting reload it would pop up immediately. Presumably there's some caching going on where subsequent requests finish within the time limit. In fact, I just now got it to load after reloading a few times. 😆

crlf0710 commented 5 years ago

Current state: file load fine, but blame still doesn't work.

Centril commented 5 years ago

I'm fixing this permanently today.