Stream: t-compiler/major changes

Topic: Move the compiler to a new `compiler/` di… compiler-team#336


triagebot (Jul 28 2020 at 03:45, on Zulip):

A new proposal has been announced: #336. It will be announced at the next meeting to try and draw attention to it, but usually MCPs are not discussed during triage meetings. If you think this would benefit from discussion amongst the team, consider proposing a design meeting.

mark-i-m (Jul 28 2020 at 03:47, on Zulip):

I would like to propose that we resolve some of the unresolved questions before the proposal is seconded

mark-i-m (Jul 28 2020 at 04:14, on Zulip):

Oh, another question: should we add nested src/ directories? I feel like the answer is yes for consistency with library/

mark-i-m (Jul 28 2020 at 04:39, on Zulip):

Slightly more out there: what do people think of removing the rustc_ prefixes of all the crates? i.e. we would have crates ast, span, mir, lexer, middle, lint, traits, etc...

mark-i-m (Jul 28 2020 at 04:41, on Zulip):

Just playing around here: https://github.com/rust-lang/rust/pull/74862

It's surprisingly easy to move most of the crates -- it's just updating the Cargo.toml. The LLVM stuff could be tricky because it interacts with bootstrap, though.

mark-i-m (Jul 28 2020 at 05:13, on Zulip):

mark-i-m said:

Slightly more out there: what do people think of removing the rustc_ prefixes of all the crates? i.e. we would have crates ast, span, mir, lexer, middle, lint, traits, etc...

Hmm... actually this would require writing the imports in every file... so maybe not...

matklad (Jul 28 2020 at 06:31, on Zulip):

Organizationally, I think it makes sense to start with moving llvm-project to the top. It is somewhat orthogonal to the proposal, but I think is clearly desirable for a very tangible benefit of easier grepping, etc (and not just to make structure more sane). It will also probably be the most painful one, with respect to fixing paths

matklad (Jul 28 2020 at 06:35, on Zulip):

Looking at the PR, another option to move everything but conpiler out of src first, and then rename the dir.

RalfJ (Jul 28 2020 at 06:45, on Zulip):

Wow this will be so painful for all ongoing parallel development.^^ It will also make code archeology so much more annoying for the next few years as "git blame" is basically unable to follow renames. I hope y'all are convinced that such a hugely disruptive change is really worth it. IMO the current structure isn't great, but it also not so bad to justify so much pain. (Moving submodules is particularly painful in my experience, in particular huge ones like llvm-project.)

oli (Jul 28 2020 at 07:29, on Zulip):

I've never had problems with git blame and renames. I recently blamed a test from pre 1.0 and it definitely has moved a few times since then. The only thing that breaks is github's "file history", but even github's blame still works correctly

Tshepang Lekhonkhobe (Jul 28 2020 at 07:34, on Zulip):

mark-i-m said:

Slightly more out there: what do people think of removing the rustc_ prefixes of all the crates? i.e. we would have crates ast, span, mir, lexer, middle, lint, traits, etc...

am a fan of removing the prefix, provided it isn't too painful... maybe can be automated with help of sd and similar

oli (Jul 28 2020 at 07:35, on Zulip):

mark-i-m said:

mark-i-m said:

Slightly more out there: what do people think of removing the rustc_ prefixes of all the crates? i.e. we would have crates ast, span, mir, lexer, middle, lint, traits, etc...

Hmm... actually this would require writing the imports in every file... so maybe not...

we could start with just renaming the directories but keeping the crate names

matklad (Jul 28 2020 at 08:14, on Zulip):

IMO the current structure isn't great, but it also not so bad to justify so much pain.

The current pain accumulates over time, the pain from reorg is temporary (provided that there's fixed point to reorgs). If I take "over time" effects into account, for me personally it is very clear that build process simplification (of which runtime/compiler split is a per-requisite) is worth the pain. Though, I am spoiled by working in the rust-analyzer code base, which is just a big boring stable Cargo workspace :D

matklad (Jul 28 2020 at 08:17, on Zulip):

Unresolved question: there was also some questions about what to do with the current rustc crate (effectively, the main.rs of the compiler), which is basically just a main function that calls rustc_driver. In the above proposal, I have it as bin/rustc.rs.

Remove the rustc crate and move that file over to librustc_driver/src/bin/rustc.rs

RalfJ (Jul 28 2020 at 09:41, on Zulip):

oli said:

I've never had problems with git blame and renames. I recently blamed a test from pre 1.0 and it definitely has moved a few times since then. The only thing that breaks is github's "file history", but even github's blame still works correctly

that is not my experience, github's blame has stopped on renames many times for me

RalfJ (Jul 28 2020 at 09:42, on Zulip):

submodule renames do not lead to the old submodule being deleted, you have to manually delete the old dir and the data in .git/modules , I think.

RalfJ (Jul 28 2020 at 09:42, on Zulip):

but it seems like rebase handles renames so at least that part should not be as bad as I thought

mark-i-m (Jul 28 2020 at 15:16, on Zulip):

oli said:

mark-i-m said:

mark-i-m said:

Slightly more out there: what do people think of removing the rustc_ prefixes of all the crates? i.e. we would have crates ast, span, mir, lexer, middle, lint, traits, etc...

Hmm... actually this would require writing the imports in every file... so maybe not...

we could start with just renaming the directories but keeping the crate names

Are you proposing that we would over time remove the prefixes from the names and fix imports? It would be weird for the crate and directory to have different names; that kind of goes against the objective of making things a bit more idiomatic

mark-i-m (Jul 28 2020 at 15:17, on Zulip):

matklad said:

Unresolved question: there was also some questions about what to do with the current rustc crate (effectively, the main.rs of the compiler), which is basically just a main function that calls rustc_driver. In the above proposal, I have it as bin/rustc.rs.

Remove the rustc crate and move that file over to librustc_driver/src/bin/rustc.rs?

@Vadim Petrochenkov proposed making it rustc/main.rs, which I kind of like

mark-i-m (Jul 28 2020 at 15:19, on Zulip):

RalfJ said:

submodule renames do not lead to the old submodule being deleted, you have to manually delete the old dir and the data in .git/modules , I think.

Yeah, submodules are a bit crazy and painful in general though, and I don't think I've ever needed to browse blame into a submodule as opposed to just browsing the blame in the submodule repo directly...

mark-i-m (Jul 28 2020 at 15:21, on Zulip):

matklad said:

Organizationally, I think it makes sense to start with moving llvm-project to the top. It is somewhat orthogonal to the proposal, but I think is clearly desirable for a very tangible benefit of easier grepping, etc (and not just to make structure more sane). It will also probably be the most painful one, with respect to fixing paths

Is anyone opposed to doing this in 2 separate PRs (i.e. one to move llvm-project, llvmrust, rustc_llvm somewhere; one to move rustc_<everything else> to compiler/)?

mark-i-m (Jul 28 2020 at 15:22, on Zulip):

More broadly, it seems reasonable for llvm-project, llvmrust, rustc_llvm to live together somewhere, but having them all in the root seems bad. Should we move them all into llvmrust at the root? any thoughts?

mark-i-m (Jul 28 2020 at 15:40, on Zulip):

Also, does anyone have opinions about moving the compiler tests? I think @Vadim Petrochenkov had expressed some concerns. (Perhaps moving tests is a 3rd PR?)

matklad (Jul 28 2020 at 15:41, on Zulip):

@mark-i-m what exactly are "compiler tests"?

matklad (Jul 28 2020 at 15:42, on Zulip):

Things like src/test/ui? I think they should be treated not like compiler tests, but like "implementation conformance tests", and be somewhat independent of particular compiler implementation.

pnkfelix (Jul 28 2020 at 15:49, on Zulip):

@matklad Its a good question. compiler-team#336 says "The src/test directory is split up to compiler/test and src/test",

pnkfelix (Jul 28 2020 at 15:49, on Zulip):

I would personally say that the format of our diagnostics is a property of a specific compiler implementation

pnkfelix (Jul 28 2020 at 15:50, on Zulip):

and therefore src/test/ui, at least as originally envisaged, would be compiler-specific

pnkfelix (Jul 28 2020 at 15:51, on Zulip):

but there's probably a gradient of perspectives here

pnkfelix (Jul 28 2020 at 15:52, on Zulip):

certainly things like the mir-opt tests that definitely inspecting compiler-internal details should be considered compiler-specific. But that's in src/test/mir-opt, of course.

mark-i-m (Jul 28 2020 at 18:09, on Zulip):

In my mind, I was thinking everything in src/test/ except for the rustdoc tests (src/test/rustdoc*)... Basically, if you change the compiler and it could impact the output of the test, it seems like that is a "compiler test"... but maybe I'm not thinking about some cases?

mark-i-m (Jul 28 2020 at 18:09, on Zulip):

I'm open to tests being somewhere else, though

Vadim Petrochenkov (Jul 28 2020 at 18:23, on Zulip):

mark-i-m said:

Slightly more out there: what do people think of removing the rustc_ prefixes of all the crates? i.e. we would have crates ast, span, mir, lexer, middle, lint, traits, etc...

Many of the crate names are too generic for that, IMO.

Vadim Petrochenkov (Jul 28 2020 at 18:24, on Zulip):

Even those that are compiler related (e.g. rustc_parse) and not just generic (rustc_data_structures).

Vadim Petrochenkov (Jul 28 2020 at 18:25, on Zulip):

I like it as is, personally.

mark-i-m (Jul 28 2020 at 19:26, on Zulip):

I guess my intuition is that if all crates have a rustc_ prefix, it doesn't seem to add much beyond having everything in a compiler/ directory... For example, the name ty doesn't seem any more or less descriptive than rustc_ty to me...

matklad (Jul 28 2020 at 20:18, on Zulip):

Procedurally, removing rustc_ prefix would require a lot of churn (and discussion), so it seems prudent to just table this discussion regardless of our opinions :-) Even if we do want to change all the imports, it's better done in a separate commit, to not mix rename & change together.

Also, don't listen to me about this: in rust-analyzer, half of the crates have ra_ prefix and half don't :D

mark-i-m (Aug 03 2020 at 18:19, on Zulip):

Ok, so for now, let's just remove the lib* prefix for now.

triagebot (Aug 15 2020 at 13:57, on Zulip):

@T-compiler: Proposal #336 has been seconded, and will be approved in 10 days if no objections are raised.

mark-i-m (Aug 15 2020 at 21:46, on Zulip):

I'm going to propose that we do two separate fcps: first for the existing PR and then for the remainder of the stuff mentioned in the MCP (tests, llvm, etc)

triagebot (Aug 26 2020 at 20:14, on Zulip):

This proposal has been accepted: #336.

DPC (Sep 03 2020 at 15:50, on Zulip):

since mark is "taking a break", i'll be taking over any work that's involved with this :smile:

Tshepang Lekhonkhobe (Sep 04 2020 at 03:06, on Zulip):

nice suggestions from this topic... rename library to lib and compiler to rustc, and remove rustc_ from dirs in rustc/

mark-i-m (Sep 04 2020 at 14:05, on Zulip):

There was some discussion earlier in the thread about removing the rustc_ prefixes and it sounded like that was a major undertaking for another time

mark-i-m (Sep 04 2020 at 14:05, on Zulip):

Because you would have to update all of the imports everywhere (unless you just wanted to change the directory without changing the crate name, which could also be reasonable)

matklad (Sep 04 2020 at 14:07, on Zulip):

rename library to lib and compiler to rustc

If we rename compiler to rustc, then library should become std, as per "either both are proper nouns or both a common nouns"

cuviper (Sep 04 2020 at 14:16, on Zulip):

Are you suggesting we'd have std/std? Or keep std/src at the top and only nest the other crates?

matklad (Sep 04 2020 at 14:18, on Zulip):

std/std

Josh Triplett (Sep 04 2020 at 16:40, on Zulip):

I feel like that would be confusing. Not everything in library corresponds to std, right?

Joshua Nelson (Sep 04 2020 at 16:40, on Zulip):

it also has the runtime I think

mark-i-m (Sep 04 2020 at 23:16, on Zulip):

There's discussion about this in the gh thread for the PR too...

DPC (Sep 04 2020 at 23:23, on Zulip):

just realised that if we changed folder names, and kept the name in cargo.toml the same as the old one, we would have touch 0 imports in other places and thus have less conflicts but it's going to be confusing and a pain at some stage later :grinning:

DPC (Sep 07 2020 at 01:22, on Zulip):

created an issue for renaming compiler crates: https://github.com/rust-lang/rust/issues/76425

est31 (Sep 19 2020 at 02:02, on Zulip):

I quite like the fact that tests aren't contained in compiler. My main tool to search in the compiler is ripgrep and tests would add tons of noise in some cases. Has it been considered to leave tests inside src/test or move them to a top-level test/ directory?

est31 (Sep 19 2020 at 02:05, on Zulip):

RalfJ said:

Wow this will be so painful for all ongoing parallel development.^^ It will also make code archeology so much more annoying for the next few years as "git blame" is basically unable to follow renames. I hope y'all are convinced that such a hugely disruptive change is really worth it. IMO the current structure isn't great, but it also not so bad to justify so much pain. (Moving submodules is particularly painful in my experience, in particular huge ones like llvm-project.)

Have you tried adding the --follow flag to git blame?

mark-i-m (Sep 19 2020 at 03:13, on Zulip):

est31 said:

I quite like the fact that tests aren't contained in compiler. My main tool to search in the compiler is ripgrep and tests would add tons of noise in some cases. Has it been considered to leave tests inside src/test or move them to a top-level test/ directory?

Yes, the same with llvm. That's why the original PR left them where they are for now... I don't think anything has been decided yet

RalfJ (Oct 04 2020 at 11:08, on Zulip):

est31 said:

RalfJ said:

Wow this will be so painful for all ongoing parallel development.^^ It will also make code archeology so much more annoying for the next few years as "git blame" is basically unable to follow renames. I hope y'all are convinced that such a hugely disruptive change is really worth it. IMO the current structure isn't great, but it also not so bad to justify so much pain. (Moving submodules is particularly painful in my experience, in particular huge ones like llvm-project.)

Have you tried adding the --follow flag to git blame?

there's no way to do that in the GH UI, is there? I rarely use git blame directly.

Last update: May 07 2021 at 07:15UTC