Stream: t-compiler/major changes

Topic: Rename `rustc_ty` compiler-team#387


triagebot (Nov 18 2020 at 18:44, on Zulip):

A new proposal has been announced: Rename rustc_ty #387. 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.

Jonas Schievink (Nov 18 2020 at 18:47, on Zulip):

what should it be renamed to? rustc_ty_utils? does any part of the compiler directly use that crate or is it just used through queries?

Léo Lanteri Thauvin (Nov 18 2020 at 18:49, on Zulip):

It looks like only rustc_interface directly depends on it

Léo Lanteri Thauvin (Nov 18 2020 at 18:50, on Zulip):

And only to call its provide function

Léo Lanteri Thauvin (Nov 18 2020 at 18:52, on Zulip):

Jonas Schievink said:

what should it be renamed to? rustc_ty_utils?

rustc_ty_utils is also what I had in mind

Joshua Nelson (Nov 18 2020 at 19:00, on Zulip):

is there any reason not to make the ty module you're extracting out of rustc_middle also include what's currently rustc_ty?

Joshua Nelson (Nov 18 2020 at 19:00, on Zulip):

or maybe just move it into rustc_interface directly?

Jack Huey (Nov 18 2020 at 19:19, on Zulip):

rustc_ty depends on rustc_middle. rustc_ty_library (or rustc_ty in the future) will be the opposite: rustc_middle will depend on it

Camelid (Nov 18 2020 at 19:32, on Zulip):

Side note: hopefully extracting the type library will speed up bootstrap times!

Jack Huey (Nov 18 2020 at 19:56, on Zulip):

That's an added bonus, yes

Léo Lanteri Thauvin (Nov 18 2020 at 20:56, on Zulip):

@Joshua Nelson The end goal is to have a library that’s independent from rustc, this rustc_ty_library is just a step forward

Joshua Nelson (Nov 18 2020 at 21:00, on Zulip):

Wait I'm confused, how can rustc_ty (the new one you're proposing) be independent if it depends on rustc_middle?

Léo Lanteri Thauvin (Nov 18 2020 at 21:03, on Zulip):

The whole thing is to make it independent :big_smile:

Joshua Nelson (Nov 18 2020 at 21:03, on Zulip):

I am really not following the proposal then, sorry. What is it suggesting and what is the goal?

Léo Lanteri Thauvin (Nov 18 2020 at 21:04, on Zulip):

Also to be clear it’s not what I’m proposing, it’s what’s been on the WG-traits’s mind for a long time

Léo Lanteri Thauvin (Nov 18 2020 at 21:04, on Zulip):

This proposal is just proposing to rename the current rustc_ty to something else

Joshua Nelson (Nov 18 2020 at 21:05, on Zulip):

If the long term goal is to have a shared library for data types, maybe it should be named rustc_ty_data

Joshua Nelson (Nov 18 2020 at 21:06, on Zulip):

And then rustc_ty doesn't need to be renamed

Joshua Nelson (Nov 18 2020 at 21:06, on Zulip):

Or there could be rustc_ty_utils and rustc_ty_data maybe, but turning one crate into another sounds very confusing

Léo Lanteri Thauvin (Nov 18 2020 at 21:07, on Zulip):

Also honestly I think rustc_ty is a confusing name (for the current library) regardless

Joshua Nelson (Nov 18 2020 at 21:17, on Zulip):

yeah changing it to rustc_ty_utils sounds like an all-around win now that I know what it does

Jack Huey (Nov 18 2020 at 21:52, on Zulip):

@Joshua Nelson if you're still confused, let me see if I can clarify

Jack Huey (Nov 18 2020 at 21:55, on Zulip):

So, the eventual goal is to have Chalk and rustc have a shared type library (and it'll will probably look close to what chalk-ir has today). While we haven't completely decided yet where exactly that will live, wg-traits has decided to start moving things out of rustc_middle into an independent crate and rustc_middle will depend on it.

nikomatsakis (Nov 19 2020 at 16:58, on Zulip):

Hmm

nikomatsakis (Nov 19 2020 at 16:58, on Zulip):

rustc_ty feels like the name I expect but I hear folks saying it's confusing

Joshua Nelson (Nov 19 2020 at 16:59, on Zulip):

well the confusing thing is that there's already a crate with that name

simulacrum (Nov 19 2020 at 17:00, on Zulip):

I was going to suggest that a general type library could just be rust_ty (note: no c) but that seems like it'd be very confusing

simulacrum (Nov 19 2020 at 17:00, on Zulip):

(since presumably the same general types could be used in rust-analyzer and such, as we explicitly aim at "generality" here)

nikomatsakis (Nov 19 2020 at 17:01, on Zulip):

I was wondering about the rustc_ prefix

nikomatsakis (Nov 19 2020 at 17:01, on Zulip):

this is aiming to be a "reusable abstraction", but ultimately it is "the rust compiler's representation of types"

nikomatsakis (Nov 19 2020 at 17:01, on Zulip):

so rustc_ seems ok

nikomatsakis (Nov 19 2020 at 17:02, on Zulip):

Joshua Nelson said:

well the confusing thing is that there's already a crate with that name

what does this crate do today?

nikomatsakis (Nov 19 2020 at 17:02, on Zulip):

it's also true that this library will cover things that are not types

nikomatsakis (Nov 19 2020 at 17:02, on Zulip):

e.g., trait references

nikomatsakis (Nov 19 2020 at 17:02, on Zulip):

so maybe there's a more general name, but I'm not sure what it is

nikomatsakis (Nov 19 2020 at 17:02, on Zulip):

"terms"

Jack Huey (Nov 19 2020 at 20:30, on Zulip):

Okay, so let's make a solid plan going forward. It sounds like there's good support here to rename rustc_ty to rustc_ty_utils. Can someone second this MCP? And @Léo Lanteri Thauvin maybe make a PR for that?

Jack Huey (Nov 19 2020 at 20:31, on Zulip):

@nikomatsakis can you look at #79169, particularly in regards to the rustc_data_structures comment

triagebot (Nov 19 2020 at 20:31, on Zulip):

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

Jonas Schievink (Nov 19 2020 at 20:31, on Zulip):

rustc_ty_utils seems reasonable and uncontroversial, seconded

Léo Lanteri Thauvin (Nov 19 2020 at 20:42, on Zulip):

Filed #79212

Jonas Schievink (Nov 19 2020 at 20:43, on Zulip):

(I don't even think this really needs an MCP if the only use is in rustc_interface)

Jonas Schievink (Nov 19 2020 at 20:54, on Zulip):

Okay yeah that diff is tiny, and according to https://forge.rust-lang.org/compiler/mcp.html#what-constitutes-a-major-change no MCP is required. I'll r+ once CI is fixed.

Léo Lanteri Thauvin (Nov 19 2020 at 21:02, on Zulip):

Should I just close the MCP?

Jonas Schievink (Nov 19 2020 at 21:04, on Zulip):

I think so?

nikomatsakis (Nov 19 2020 at 21:37, on Zulip):

Jack Huey said:

can you look at #79169, particularly in regards to the rustc_data_structures comment

I saw it. I feel like that crate probably wants to be broken up.

Jacob Lifshay (Nov 19 2020 at 22:07, on Zulip):

how about naming it rust-typesystem?

Jack Huey (Nov 19 2020 at 22:07, on Zulip):

@nikomatsakis yeah, I feel like eventually we should split out a minimal set of things needed from that crate, and make it a dependency of chalk-ir

nikomatsakis (Nov 20 2020 at 15:12, on Zulip):

Jacob Lifshay said:

how about naming it rust-typesystem?

I...kind of like this

nikomatsakis (Nov 20 2020 at 15:12, on Zulip):

it's not really the whole type system but it is definitely the "IR" for the type system

nikomatsakis (Nov 20 2020 at 15:12, on Zulip):

type_ir might be another possible name

Jack Huey (Dec 02 2020 at 00:36, on Zulip):

Okay, so I'm trying to think about how to move forward with this. Specifically with regards to rustc_data_structures and other potential new required dependencies for chalk-ir

Jack Huey (Dec 02 2020 at 00:37, on Zulip):

So, from rustc_data_structures, we will probably need HashStable. I don't know what else.

Jack Huey (Dec 02 2020 at 00:38, on Zulip):

But, we also probably need at least some of rustc_serialize because of TyEncodable/TyDecodable

Joshua Nelson (Dec 02 2020 at 00:38, on Zulip):

IIRC the ObligationForest is only used in rustc_trait_selection, right? Does it make sense to move it there?

Jack Huey (Dec 02 2020 at 00:39, on Zulip):

Well, I don't know how far we'll get trying to move things out of rustc_data_structures into other parts of rustc

Jack Huey (Dec 02 2020 at 00:40, on Zulip):

If anything, I imagine we'll probably want to move like a minimal set of things to a crate that chalk-ir depends on

Jack Huey (Dec 02 2020 at 00:40, on Zulip):

If HashStable is the only thing we need from there, that's easy

Jack Huey (Dec 02 2020 at 00:40, on Zulip):

FxHashSet/FxHashMap are actually in rustc_hash, and we already depend on that

Jack Huey (Dec 02 2020 at 00:41, on Zulip):

Something farther down the line to maybe thing about is ParamEnv, which uses CopyTaggedPtr

Jack Huey (Dec 02 2020 at 00:50, on Zulip):

Actually...could we just move HashStable into rustc_hash?

Jack Huey (Dec 02 2020 at 00:51, on Zulip):

It shouldn't make any difference for rustc itself, since everything refers to it through rustc_data_structure reexports

Jack Huey (Dec 02 2020 at 00:52, on Zulip):

It would make anyone who is currently using rustc_hash start to pull in a bit more

Jack Huey (Dec 02 2020 at 00:52, on Zulip):

but, that's probably okay

Joshua Nelson (Dec 02 2020 at 00:53, on Zulip):

huh, there's a surprising number of people using rustc_hash https://crates.io/crates/rustc_hash/reverse_dependencies

Joshua Nelson (Dec 02 2020 at 00:53, on Zulip):

but like, I wouldn't expect HashStable to be giant, if it is we can always make it a cargo feature or something

Jack Huey (Dec 02 2020 at 00:56, on Zulip):

huh TIL about that reverse deps feature

Joshua Nelson (Dec 02 2020 at 00:56, on Zulip):

yeah it's useful :)

Jack Huey (Dec 02 2020 at 00:56, on Zulip):

Just looking at the top ones:

Jack Huey (Dec 02 2020 at 00:57, on Zulip):

hashbrown only uses it in a test

Jack Huey (Dec 02 2020 at 00:57, on Zulip):

bindgen actually uses it

Jack Huey (Dec 02 2020 at 00:58, on Zulip):

Anyways, yeah, I'm gonna go ahead and open a PR for this. I'll put it under a feature gate,

Jack Huey (Dec 02 2020 at 01:07, on Zulip):

Oh but rustc_hash is so tiny :( and not even in-tree

Jack Huey (Dec 02 2020 at 17:29, on Zulip):

So, who actually "maintains" rustc-hash? I.e. can make the decision whether or not it's okay to expand it to include the hash_stable mod from rustc_data_structures?

Léo Lanteri Thauvin (Dec 02 2020 at 17:29, on Zulip):

According to crates.io it's T-core and T-compiler

Léo Lanteri Thauvin (Dec 02 2020 at 17:30, on Zulip):

Do you think this warrants a T-compiler MCP?

Jack Huey (Dec 02 2020 at 17:33, on Zulip):

Maybe?

Jack Huey (Dec 02 2020 at 17:35, on Zulip):

As much as I like the MCP process, I feel like to a certain extent, we should maybe just get a bit broader approval for the ty library related things. We've already sort of discussed it at length in wg-traits. This in particular seems to be more a "just check in with relevant people" sort of thing. But I don't know

Jack Huey (Dec 02 2020 at 17:36, on Zulip):

cc @nikomatsakis since you did the initial commits of rustc-hash, and are obviously relevant for everything else

Jack Huey (Dec 02 2020 at 17:37, on Zulip):

In the meantime, @Léo Lanteri Thauvin can you adjust your type library PR for two things:

Jack Huey (Dec 02 2020 at 17:37, on Zulip):
Jack Huey (Dec 02 2020 at 17:38, on Zulip):
Léo Lanteri Thauvin (Dec 02 2020 at 17:38, on Zulip):

Sure

Jack Huey (Dec 02 2020 at 17:38, on Zulip):

I figure that'll get split at some point in some way, but for now it's fine to start

Jack Huey (Dec 02 2020 at 17:41, on Zulip):

(This next statement is opinion and not necessarily what we should do) You could also reexport all of that library under rustc_middle::ty::ir

Jack Huey (Dec 02 2020 at 17:42, on Zulip):

So instead of ty::INNERMOST you could do ty::ir::INNERMOST

Jack Huey (Dec 02 2020 at 17:42, on Zulip):

(Though you could just as easily reexport at rustc_middle::ty and not change most code)

Léo Lanteri Thauvin (Dec 02 2020 at 17:43, on Zulip):

Jack Huey said:

So instead of ty::INNERMOST you could do ty::ir::INNERMOST

Seems a bit lengthy to me though, but :shrug:

Jack Huey (Dec 02 2020 at 17:43, on Zulip):

Yeah, that's my concern too. For now, can you just reexport all at rustc_middle::ty?

Jack Huey (Dec 02 2020 at 17:43, on Zulip):

So almost all code doesn't change

Jack Huey (Dec 02 2020 at 17:44, on Zulip):

Then at some future date, we can remove that reexport

Jack Huey (Dec 02 2020 at 20:55, on Zulip):

@nikomatsakis if you get a chance, can you take a look at #79169

Last update: May 07 2021 at 07:00UTC