Rust: Split up files with `// ignore-tidy-filelength`

[varkor]

These files are over 3,000 lines, which is not ideal for navigating or comprehending. If we can, it would be good to split these up. See also https://github.com/rust-lang/rust/issues/60015, which is a specific issue of this problem.

This issue has been assigned to @alex-griffiths via this comment.

[petrochenkov]

https://github.com/rust-lang/rust/issues/61097 is going to help with some of the files.

[olamiko]

Which files @varkor? The files in #60015 are done

[varkor]

I count 17 different files that still have this comment.

[alex-griffiths]

Hey, I'm not sure if anyone is working on this, but, this looks like it might be a good place to get started contributing.

I am quite new to the language, but I'd like to get involved. Is this something as simple as pulling out structs and other related blocks of code into their own files and then including them in the original?

[varkor]

@alex-griffiths: yes, it should be as simple as figuring out some sensible way to split up large files into related types and implementations!

[alex-griffiths]

@varkor Alright, mind if I take a crack at this?

[varkor]

@alex-griffiths: yes, go ahead! You can claim the issue with:

@rustbot claim

[alex-griffiths]

@rustbot claim

[alex-griffiths]

@varkor I'm finding that a lot of the files that include the // ignore-tidy-filelength comment are files that include lots of the documentation.

I'm still kind of new to the language, but for example in src/libstd/fs.rs there are 6 structs where the struct definition itself is a single line, but with varying amounts of docs. Is this the kind of thing that is able to be extracted into it's own file which is then included as a module in the fs.rs file?

[varkor]

@alex-griffiths: that's an interesting observation. I think in these cases, it's not so important to split up the files, because the length doesn't pose an issue for comprehension. Anyone editing the file is going to be interested in one or two particular functions, which are well-isolated within the file, and wouldn't benefit from being in their own file. In some sense, files like these are "false positives", though I don't think we could automatically exclude them. The lint is really more for files with large functions, and interdependency that we want to make easier to navigate. Thanks for pointing this out!

[alex-griffiths]

I'll start working through all the files that have the comment in them, and I'll compile a list of files that appear to mostly documentation, and then for the rest, I'll start trying to improve the navigability of them.

[danii]

Would like to help with this, too. Is there a possibility to share an issue?
Specifically, I have my eyes on core::num.

[alex-griffiths]

Yeah I'm happy with this @varkor.

Perhaps we could do something similar to how #75080 is managing the multiple people doing for that issue?

[jyn514]

Seems like a good idea. You need

• a list of the files to be split up
• instructions for how to split them up (more detailed is better, but simple is fine as long as you update it along the way)
• someone to keep the tracking issue up to date as people claim files. Maybe in this case that would be @varkor?

[danii]

Oh sweet, thanks! This'll be my first contributions to the rust standard library. 😄
I personally don't think we'll need instructions on splitting the files up as long as everyone's respectful. Although, it is @alex-griffiths decision.
If no one minds I'm gonna list out the files that need truncating.

[danii]

Claimed By @alex-griffiths

PR: #75701

• alloc::vec
• core::str

Claimed By @lzutao

PR: #76325

• core::slice
• core::num

Claimed By @daniihh

PR: #76391

• core::tests::iter

Claimed By @nixphix

PR: #76426

• rustc_apfloat::tests::ieee

Unclaimed

• src/test/ui/issues/issue-29466.rs
• src/test/ui/issues/issue-72933-match-stack-overflow.rs
• core::iter::traits::iterator
• rustc_middle::ty
• compiletest::runtest
• alloc::collections::vec_deque
• src/test/ui/issues/issue-74564-if-expr-stack-overflow.rs
• rustdoc::html::render

Completed

• rustc_typeck::check::fn_ctxt

Out Of Scope

• rustc_resolve per https://github.com/rust-lang/rust/pull/75701#issuecomment-676266089

[alex-griffiths]

I believe changes to rustc_resolve need to be handled in a different manner.
https://github.com/rust-lang/rust/pull/75701#issuecomment-676266089

[lzutao]

I am doing core::slice splitting (#76311 )

[lzutao]

Hi @alex-griffiths, Would you mind if I take care of core::str part ?

[jyn514]

@lzutao maybe do core::num first since alex is in the middle of splitting core::str

[lzutao]

Yeah, he also did alloc/vec.rs in #75701 . But he seems having trouble in splitting core/str.rs.
I would like to help him a hand.

Edit: It's up as a draft PR #76325

[lzutao]

I'm doing core::num.

[danii]

I've finally got time, guess I'll be doing core::tests::iter.
Or is it core::tests::iter? I checked and apparently it's in another target other than lib, and I don't know how to represent that, lol.

Draft: #76391

[nixphix]

I ll work on rustc_apfloat::tests::ieee

[lzutao]

Is there any reviewers want to review big PR: #76327 ?
If not, I'm going to close it if it'd have merge conflicts again.

[Nicholas-Baron]

I will try my hand at rustc_typeck::check.

[Nicholas-Baron]

@Daniihh The issue is no longer with rustc_typeck::check, but with rustc_typeck::check::fn_ctxt.

The FnCtxt struct that it houses has a lot of functionality wrapped into it (the struct {} alone taking 85 lines w/documentation).

The more permanent solution to its length issue is to (in my opinion) move some fields out to "sub-structs".

[lzutao]

Could somebody review #76325 ? It stands still 19+ days without review.

[Nicholas-Baron]

@danii My pull request for FnCtxt was merged. Take my name off the list, please.

[DeveloperC286]

@varkor Do you or someone want to take a look at my PR https://github.com/rust-lang/rust/pull/78590 it has been 11 days and no comments so far, thanks.