Stream: t-compiler

Topic: polymorphization estimates


nikomatsakis (Dec 14 2018 at 16:05, on Zulip):

@davidtwco chat in about 30 minutes?

davidtwco (Dec 14 2018 at 16:24, on Zulip):

@nikomatsakis sure

nikomatsakis (Dec 14 2018 at 16:26, on Zulip):

ok gimme a few minutes

nikomatsakis (Dec 14 2018 at 18:00, on Zulip):

ok @davidtwco the branch we were working on is here:

https://github.com/nikomatsakis/rust/tree/polymorphize-analysis

it includes the (now passing) test. I'd say the next steps are to

well that and maybe write some comments. =)

davidtwco (Dec 14 2018 at 18:10, on Zulip):

Sounds good, will try to take a look at it tonight.

nikomatsakis (Dec 14 2018 at 18:24, on Zulip):

those two steps seem pretty independent

davidtwco (Dec 15 2018 at 11:20, on Zulip):

Added a commit on my copy of the branch (no permissions on yours) for the depend_size_alignment test - I suspect however that it is slightly too specific to that test.

davidtwco (Dec 15 2018 at 16:02, on Zulip):

Added a commit that tidies that up slightly and has a first - probably naive and inefficient - attempt at the inter-procedural propagation.

nikomatsakis (Dec 17 2018 at 15:32, on Zulip):

@davidtwco invited you as a collaborator

nikomatsakis (Dec 17 2018 at 15:32, on Zulip):

I can pull your commits tho

davidtwco (Dec 17 2018 at 15:32, on Zulip):

Great, accepted that.

davidtwco (Dec 17 2018 at 15:33, on Zulip):

Double check they're what we're looking for before pulling them :P

nikomatsakis (Dec 17 2018 at 15:34, on Zulip):

I pulled them, but I'll check them out now :)

nikomatsakis (Dec 17 2018 at 15:34, on Zulip):

or, soon

nikomatsakis (Dec 17 2018 at 15:51, on Zulip):

@davidtwco so I think what you want to be doing in this code is using the SizeSkeleton::compute function. We can check the return type: if it is an Ok result with Known, then there is no dependency.

It's a bit tricky to decide what to do otherwise. For now, we could use the existing record_dependency logic on the "unknown parts", but it's not really as precise as we might like. Good enough for now.

davidtwco (Dec 17 2018 at 15:53, on Zulip):

Thanks, I'll get to that a little later today.

nikomatsakis (Dec 17 2018 at 15:53, on Zulip):

In particular, i'd like to identify the cases where "all we need to know is the size/alignment of the parameter", but that's actually a bit tricky.

For example, if we have a value of type &T::Item, and T::Item is not known to be sized, we would get back a [SizeSkeleton::Pointer](https://doc.rust-lang.org/nightly/nightly-rustc/rustc/ty/layout/enum.SizeSkeleton.html#variant.Pointer) variant that says we "just need to know T::Item". But knowing the size/alignment of T` alone is not helpful.

nikomatsakis (Dec 17 2018 at 15:53, on Zulip):

But that's ok

nikomatsakis (Dec 17 2018 at 15:53, on Zulip):

Let's start with just capturing the higher-level stuff and then we'll try to separate out the cases

nikomatsakis (Dec 17 2018 at 15:54, on Zulip):

basically we'll be interested in some special cases where just knowing the size/align would be enough, and the rest would be some more general case

nikomatsakis (Dec 17 2018 at 15:54, on Zulip):

(really it also depends on how much work we'd be willing to put into doing layout code at runtime)

nikomatsakis (Dec 17 2018 at 15:54, on Zulip):

anyway let me look at the next commit now

davidtwco (Dec 17 2018 at 15:55, on Zulip):

I suspect the propagation isn't quite right but figured it was worth an attempt - first time trying to implement something like that from scratch.

nikomatsakis (Dec 17 2018 at 15:55, on Zulip):

yep, it's not

nikomatsakis (Dec 17 2018 at 15:55, on Zulip):

but it's in the right direction :)

nikomatsakis (Dec 17 2018 at 15:56, on Zulip):

btw

nikomatsakis (Dec 17 2018 at 15:56, on Zulip):

in both cases, I guess we're passing the tests we had?

nikomatsakis (Dec 17 2018 at 15:56, on Zulip):

(suggests we want some new tests)

davidtwco (Dec 17 2018 at 15:56, on Zulip):

Yeah, we pass all the tests that you wrote - including the ones that we had as //FIXME.

nikomatsakis (Dec 17 2018 at 15:57, on Zulip):

I'm a bit surprised by that

nikomatsakis (Dec 17 2018 at 15:57, on Zulip):

maybe I missed something

nikomatsakis (Dec 17 2018 at 15:59, on Zulip):

I'll do a local build and inspect

nikomatsakis (Dec 17 2018 at 16:06, on Zulip):

pushed some tests for the local stuff at least, @davidtwco, that are still failing :)

davidtwco (Dec 17 2018 at 16:08, on Zulip):

Great, I'll take a look at them.

davidtwco (Dec 17 2018 at 16:29, on Zulip):

@nikomatsakis do you have any thoughts on how I should change the propagation so it is more what you were looking for?

nikomatsakis (Dec 17 2018 at 17:00, on Zulip):

@davidtwco I want to look at why the existing tests are passing :) the main things I think are missing are:

davidtwco (Dec 17 2018 at 17:00, on Zulip):

What do you mean by processing in this case?

nikomatsakis (Dec 17 2018 at 17:18, on Zulip):

mm I mean we need to "examine" them to see whether the types in question are known

nikomatsakis (Dec 17 2018 at 17:18, on Zulip):

e.g., if we had to know the size/alignment of T, and the caller knows that T is u32, we should be all set

davidtwco (Dec 17 2018 at 17:20, on Zulip):

So, if we have a call edge to a function with a dependency - then there are two cases, it's either called with another type parameter that we don't know the size/alignment of, or it is called with a type that we know, such as u32? Do we still record a dependency in both cases, or only the former?

nikomatsakis (Dec 17 2018 at 17:26, on Zulip):

only the former

nikomatsakis (Dec 17 2018 at 17:26, on Zulip):

but the test I expected to exercise that path

nikomatsakis (Dec 17 2018 at 17:26, on Zulip):

is passing

nikomatsakis (Dec 17 2018 at 17:26, on Zulip):

and I don't know why

davidtwco (Dec 17 2018 at 17:28, on Zulip):

I remember doing something to make that pass.

davidtwco (Dec 17 2018 at 17:28, on Zulip):

But I forget what.

nikomatsakis (Dec 17 2018 at 17:29, on Zulip):

ok :) I'l try to dig in later if I can scrape up a litle time

davidtwco (Dec 17 2018 at 17:32, on Zulip):

One thing I’m not clear on is what end state we want to end up with - should we only end up with dependencies on functions that depend on some information about the type of a type parameter but not all the uses of those functions if a use provides a concrete type? I guess I mostly don’t understand how we’ll keep track that there was a use of the function with a concrete type since I feel like that would be necessary when deciding how many copies of the function to make? Or maybe I’m getting this backwards and thinking about the functions with dependencies as those where we want to polymorphize but jn fact it is the opposite - I think that’s it. I guess I’ve answered my own question.

nikomatsakis (Dec 17 2018 at 17:34, on Zulip):

should we only end up with dependencies on functions that depend on some information about the type of a type parameter but not all the uses of those functions if a use provides a concrete type?

this sounds right

nikomatsakis (Dec 17 2018 at 17:34, on Zulip):

I guess I mostly don’t understand how we’ll keep track that there was a use of the function with a concrete type since I feel like that would be necessary when deciding how many copies of the function to make

this code doesn't really have the job of deciding how many copies to make

nikomatsakis (Dec 17 2018 at 17:35, on Zulip):

it has the job of deciding which aspects of the type parameters are important

nikomatsakis (Dec 17 2018 at 17:35, on Zulip):

there is another piece of code that figures out the full set of copies that are needed (it's called the collector)

davidtwco (Dec 17 2018 at 17:35, on Zulip):

Yeah, I think I get it now. I was conflating that.

davidtwco (Dec 17 2018 at 17:40, on Zulip):

Doesn’t that mean that the collector already does some of what this code is doing? Perhaps naively, I’d assume that this collector would need to identify which functions have dependencies so it knows which will need multiple copies (assuming they’re instantiated with more than one type that has a different size/alignment or trait method impl and therefore will need multiple copies). Then it would do propagation to work out what distinct size/alignments it needs copies for?

davidtwco (Dec 17 2018 at 17:48, on Zulip):

(Assuming that the collector is some code that already exists and isn’t something we’re going to add later)

davidtwco (Dec 18 2018 at 09:42, on Zulip):

@nikomatsakis Pushed a commit with the SizeSkeleton::compute check.

davidtwco (Dec 18 2018 at 09:42, on Zulip):

Remembered that this was the fix that made the no_dependency_indirect case pass - shouldn't be needed with the fixed propagation.

davidtwco (Dec 18 2018 at 09:45, on Zulip):

I looked into trying to do the substitution and processing you mentioned but could quite work out how to do that. I expected that I'd need to take the DefId callee, substitute the Substs into that and then use the ParamTy as a sort of index to get the right type parameter that was the dependency back out as a Ty that I can then do the same size checks to decide whether to record a dependency.

davidtwco (Dec 18 2018 at 09:45, on Zulip):

Not sure if that's right but it sounded right.

davidtwco (Dec 18 2018 at 09:53, on Zulip):

(fairly certain it isn't)

davidtwco (Dec 18 2018 at 09:57, on Zulip):

Also, can you confirm that the dependency_because_embed_ref_sized function should have a dependency? - there isn't a FIXME comment like in other tests and it didn't start to error like I'd have expected - wasn't sure if there was more I had to do or if it was incorrectly named.

davidtwco (Dec 18 2018 at 10:50, on Zulip):

Ignore the above w/r/t propagation, figured it out and pushed a commit.

nikomatsakis (Dec 18 2018 at 15:25, on Zulip):

Remembered that this was the fix that made the no_dependency_indirect case pass - shouldn't be needed with the fixed propagation.

ah, I missed that line

nikomatsakis (Dec 18 2018 at 15:26, on Zulip):

I'll pull and check out the latest in a bit :)

nikomatsakis (Dec 18 2018 at 17:33, on Zulip):

@davidtwco looking now -- seems much closer! I think that the method visit_ty could use to be renamed, and doesn't look quite right.

For one thing, if the dependency is not related to size/alignment, then it doesn't matter whether we have a known size or not.

But also, maybe the idea of keying the dependency on just a TyParam is sort of flawed anyway, even for size. Maybe it'd be better to make the Dependency include either a TraitRef (indicating the method that is being invoked) or the full Ty that is being moved about, etc.

Then we can basically just implement TypeFoldable and apply the subst method to it, and then "re-add" it. In the case of a method dispatch, this would check whether the Instance is now statically known; in the case of a copy/move, it would check if the type is statically known.

nikomatsakis (Dec 18 2018 at 17:33, on Zulip):

I have a test for at least one problem.

nikomatsakis (Dec 18 2018 at 17:33, on Zulip):

I will push, and maybe try to make a few more tests to show what I'm talking about.

nikomatsakis (Dec 18 2018 at 17:34, on Zulip):

(If you want, I can do the fix I'm talking about, or you can take a stab, I'm good either way)

davidtwco (Dec 18 2018 at 17:35, on Zulip):

Feel free to, I'll take a go at whatever is left when I'm next looking at it.

davidtwco (Dec 18 2018 at 17:41, on Zulip):

But also, maybe the idea of keying the dependency on just a TyParam is sort of flawed anyway, even for size.

What do you think is flawed about it?

nikomatsakis (Dec 18 2018 at 17:48, on Zulip):

I pushed a few tests; it's at least imprecise. That said, poking a bit at my proposed alternative raised some questions and I stopped for now, since I want to look at your NLL PR first. =)

In particular, I realized that the set of call edges can grow dynamically as we get more precise, and I just wanted to think about what is the right question we should be asking precisely anyway.

davidtwco (Dec 18 2018 at 17:53, on Zulip):

Also, can you confirm that the dependency_because_embed_ref_sized function should have a dependency? - there isn't a FIXME comment like in other tests and it didn't start to error like I'd have expected - wasn't sure if there was more I had to do or if it was incorrectly named.

Just going to bump this message because it's still something I'm unsure of.

nikomatsakis (Dec 18 2018 at 22:35, on Zulip):

i'm confused about that question

nikomatsakis (Dec 18 2018 at 22:35, on Zulip):

@davidtwco I see this:

fn dependency_because_embed_ref_sized<T>(t: &T) -> EmbedRef<'_, T> {
    //~^ ERROR no polymorphic dependencies found
    //
    // Here, the size of `EmbedRef` is known up front.
    EmbedRef { t }
}
nikomatsakis (Dec 18 2018 at 22:35, on Zulip):

i.e., no dependency

nikomatsakis (Dec 18 2018 at 22:35, on Zulip):

I think this is correct because T: Sized is known

nikomatsakis (Dec 18 2018 at 22:37, on Zulip):

oh, I see

nikomatsakis (Dec 18 2018 at 22:37, on Zulip):

it is indeed ill-named

davidtwco (Dec 18 2018 at 22:39, on Zulip):

Cool, I suspected it was but wanted to check.

nikomatsakis (Dec 18 2018 at 22:45, on Zulip):

ps I'm working on the other thing I was talking about

davidtwco (Dec 18 2018 at 23:01, on Zulip):

Great, I won’t be looking at the branch until tomorrow anyway.

davidtwco (Dec 19 2018 at 18:07, on Zulip):

Do you have work-in-progress local changes for this? Not looking to do anything on it right now, just checking if you've got work locally as I might take a look later.

nikomatsakis (Dec 19 2018 at 19:02, on Zulip):

good question :)

nikomatsakis (Dec 19 2018 at 19:12, on Zulip):

I didn't get things working or bulding before I had to stop last night and I haven't had a chance to get back to it

nikomatsakis (Dec 19 2018 at 19:12, on Zulip):

I may be able to put in a bit of time later on though

nikomatsakis (Dec 19 2018 at 20:05, on Zulip):

ok @davidtwco I have a pretty inefficient thing that seems to be working

nikomatsakis (Dec 19 2018 at 20:06, on Zulip):

pushed

davidtwco (Dec 19 2018 at 20:06, on Zulip):

For making Dependency contain a TraitRef/Ty?

nikomatsakis (Dec 19 2018 at 20:06, on Zulip):

yes

nikomatsakis (Dec 19 2018 at 20:06, on Zulip):

and in general just being more precise

nikomatsakis (Dec 19 2018 at 20:06, on Zulip):

I didn't have time to write many comments :( but take a look if you get a sec

nikomatsakis (Dec 19 2018 at 20:06, on Zulip):

and we can discuss how it works

nikomatsakis (Dec 19 2018 at 20:07, on Zulip):

it fixes my newer tests anyway ;)

davidtwco (Dec 19 2018 at 20:11, on Zulip):

I think I see how it works.

davidtwco (Dec 19 2018 at 20:13, on Zulip):

You changed the error on dependency_because_unsized_pointer_indirect from "some dependencies" to "no dependencies" and removed the fixme - should that test be named no_depend_because_unsized_pointer_indirect or should there still be a FIXME?

nikomatsakis (Dec 19 2018 at 20:29, on Zulip):

test name is wrong

nikomatsakis (Dec 19 2018 at 20:29, on Zulip):

pushed fix

davidtwco (Dec 19 2018 at 20:30, on Zulip):

Cool, so what are the next steps for this then?

davidtwco (Dec 19 2018 at 20:30, on Zulip):

Try and make that more efficient or just get more test cases passing?

nikomatsakis (Dec 19 2018 at 20:35, on Zulip):

efficiency only sort of matters at this stgae, since the primary goal is to get some statistics,

nikomatsakis (Dec 19 2018 at 20:36, on Zulip):

but we do probably have to handle cross-crate correctly

nikomatsakis (Dec 19 2018 at 20:36, on Zulip):

plus I there are surely other dependencies beyond copy/move

nikomatsakis (Dec 19 2018 at 20:37, on Zulip):

e.g., accessing fields might require knowing layout of structs that might depend on type parameters

nikomatsakis (Dec 19 2018 at 22:17, on Zulip):

I can try try to make some tests for that sort of thing

nikomatsakis (Dec 20 2018 at 16:08, on Zulip):

I'm guessing @davidtwco you didn't try to do anything here, right?

davidtwco (Dec 20 2018 at 16:14, on Zulip):

Not yet, no.

nikomatsakis (Dec 26 2018 at 17:32, on Zulip):

@davidtwco I just pushed a new test, polymorphize_place

nikomatsakis (Dec 26 2018 at 17:32, on Zulip):

it includes a struct:

struct OffsetDependent<T> {
    t: T,
    count: u32,
}
nikomatsakis (Dec 26 2018 at 17:32, on Zulip):

and a function like this

fn dependency_because_offset_depends_on_T<T>(parameter: &OffsetDependent<T>) -> u32 {
    //~^ ERROR no polymorphic dependencies found
    //
    // FIXME -- the offset of the `count` field depends on size/alignment of `T`
    parameter.count
}

you can see the FIXME :)

nikomatsakis (Dec 26 2018 at 17:32, on Zulip):

this is another sort of dependency we don't yet detect

nikomatsakis (Dec 26 2018 at 17:33, on Zulip):

I guess we have to visit each Field projection in each place and check that the offset of that field is statically known

nikomatsakis (Dec 26 2018 at 17:34, on Zulip):

probably by trying to construct a Layout

davidtwco (Dec 26 2018 at 17:35, on Zulip):

Alright, I’ll give this a shot.

davidtwco (Dec 26 2018 at 18:34, on Zulip):

Do we want a new DependencyKind variant for this or should we just use SizeAlignment?

davidtwco (Dec 26 2018 at 18:41, on Zulip):

Pushed what I have for that case.

nikomatsakis (Dec 26 2018 at 18:59, on Zulip):

@davidtwco well, it feels like a new variant; e.g., I could imagine having OffsetOf(Ty, FieldIndex) or something -- saying that the offset of the given field must be known

davidtwco (Dec 26 2018 at 19:00, on Zulip):

I thought it might, will update that now.

nikomatsakis (Dec 26 2018 at 19:00, on Zulip):

it feels different than size-alignment anyway

nikomatsakis (Dec 26 2018 at 19:00, on Zulip):

since knowing the size is not enough

nikomatsakis (Dec 26 2018 at 19:00, on Zulip):

maybe worth adding a test for that

nikomatsakis (Dec 26 2018 at 19:01, on Zulip):

if we were really smart we would realize that e.g. if you flip the order of the fields in that struct, the type T doesn't even matter

nikomatsakis (Dec 26 2018 at 19:01, on Zulip):

I'm trying to think of a case where we might know the size of a struct but still not know the answer we need

davidtwco (Dec 26 2018 at 19:01, on Zulip):

And the FieldIndex here is that of the field we need the offset of, not the field that causes the problem?

nikomatsakis (Dec 26 2018 at 19:01, on Zulip):

maybe that's just not possible

davidtwco (Dec 26 2018 at 19:01, on Zulip):

(or actually, the opposite)

nikomatsakis (Dec 26 2018 at 19:02, on Zulip):

And the FieldIndex here is that of the field we need the offset of, not the field that causes the problem?

correct

nikomatsakis (Dec 26 2018 at 19:02, on Zulip):

at least that was how I thought of it

davidtwco (Dec 26 2018 at 19:02, on Zulip):

That's easier for now, so I'll do that.

nikomatsakis (Dec 26 2018 at 19:03, on Zulip):

I'm trying to think of a case where we might know the size of a struct but still not know the answer we need

I guess that to know the size of a struct, we must know the offset of all of its fields

nikomatsakis (Dec 26 2018 at 19:03, on Zulip):

so I suppose that knowing the size is sufficient, just not necessary

nikomatsakis (Dec 26 2018 at 19:04, on Zulip):

also, given that we re-order fields and things, the ordering of fields doesn't really matter

nikomatsakis (Dec 26 2018 at 19:04, on Zulip):

(unless the struct is #[repr(C)] but that is probably thinking too hard)

nikomatsakis (Dec 26 2018 at 19:04, on Zulip):

so maybe there isn't really a need for a new kind of dependency, though it also doesn't seem bad to have one

nikomatsakis (Dec 26 2018 at 19:05, on Zulip):

(so perhaps @davidtwco your commit is basically good as is)

davidtwco (Dec 26 2018 at 19:06, on Zulip):

Even if was treated the exact same as the other variant, it's nice for some context I think.

nikomatsakis (Dec 26 2018 at 19:07, on Zulip):

yes

nikomatsakis (Dec 26 2018 at 19:07, on Zulip):

I'm trying to think what other cases could cause problems now

nikomatsakis (Dec 26 2018 at 19:07, on Zulip):

I guess another example might be assign -- assigning to a local of unknown type

nikomatsakis (Dec 26 2018 at 19:08, on Zulip):

I'm just looking down the list of cases in the visitor basically

nikomatsakis (Dec 26 2018 at 19:08, on Zulip):

most cases wind up covered by Operand

nikomatsakis (Dec 26 2018 at 19:09, on Zulip):

or some combination of other things

nikomatsakis (Dec 26 2018 at 19:10, on Zulip):

though it might be good to add code for them anyway (e.g., all local variables should have types of known size)

davidtwco (Dec 26 2018 at 19:14, on Zulip):

Pushed another commit with a new variant.

nikomatsakis (Dec 26 2018 at 19:18, on Zulip):

ok well I guess the next big steps is

nikomatsakis (Dec 26 2018 at 19:19, on Zulip):

I have to run now but I'll be back some tomorrow

nikomatsakis (Dec 26 2018 at 19:19, on Zulip):

I'm not 100% sure where the collector code lives these days :P

nikomatsakis (Dec 26 2018 at 19:19, on Zulip):

looks like this rustc_mir::monomorphize::collector

nikomatsakis (Dec 26 2018 at 19:20, on Zulip):

so I think the idea would be to (presuming the option is enabled) take the full list of monomorphized items we need to create

nikomatsakis (Dec 26 2018 at 19:20, on Zulip):

take the dependency kinds we get for each fn

nikomatsakis (Dec 26 2018 at 19:20, on Zulip):

and kind of join the two together -- basically find out how many monomorphiations we could avoid

nikomatsakis (Dec 26 2018 at 19:21, on Zulip):

i.e., if we have some fn foo<T, U> and we find that it has a bunch of dependencies on T but none on U, and then we see that we make (currently) 4 copies foo<i32, i32>, foo<u32, i32>, foo<i32, u32>, and foo<u32, u32>,

nikomatsakis (Dec 26 2018 at 19:21, on Zulip):

then we would know that 2 of those copies could be avoided, and we could just make foo<i32, _> and foo<u32, _>

davidtwco (Dec 26 2018 at 19:22, on Zulip):

:thumbs_up:

nikomatsakis (Dec 26 2018 at 19:22, on Zulip):

I am debating the best way to do this :)

nikomatsakis (Dec 26 2018 at 19:23, on Zulip):

one way would be to take the dependencies for the fn, substitute the values of the type parameters, and then just have a hashset of those substituted results

nikomatsakis (Dec 26 2018 at 19:23, on Zulip):

I think that would be correct?

nikomatsakis (Dec 26 2018 at 19:23, on Zulip):

ah, I guess that we don't want to modify the collector really

nikomatsakis (Dec 26 2018 at 19:24, on Zulip):

instead, we can probably just invoke the query that returns its results

nikomatsakis (Dec 26 2018 at 19:24, on Zulip):

from inside the poloymorphization code

nikomatsakis (Dec 26 2018 at 19:24, on Zulip):

at least I think there is a query

nikomatsakis (Dec 26 2018 at 19:28, on Zulip):

ok, the query is collect_and_partition_mono_items

nikomatsakis (Dec 26 2018 at 19:29, on Zulip):

I really do have to go now :) that's...probably not quite enough crumbs @davidtwco to point you in the right direction, but I can try to leave some more details tips tomorrow :)

nikomatsakis (Dec 26 2018 at 19:29, on Zulip):

and/or maybe I'll try to do a "first draft"

nikomatsakis (Dec 27 2018 at 12:00, on Zulip):

@davidtwco pushed a new test, polymorphize_drop.rs, showing a kind of dependency we are not accounting for

nikomatsakis (Dec 27 2018 at 12:00, on Zulip):

specifically, we are not detecting when drop glue would have to be run

davidtwco (Dec 27 2018 at 12:06, on Zulip):

I think that right now if we have a struct with three fields where the second field is the type parameter (that we don’t know the size of) then the current code will add a dependency for accessing the first field even though we’d know the offset?

davidtwco (Dec 27 2018 at 12:06, on Zulip):

Is that expected?

davidtwco (Dec 27 2018 at 12:07, on Zulip):

Since it tries to compute the layout of the whole struct but won’t be able to because of the later fields.

nikomatsakis (Dec 27 2018 at 12:22, on Zulip):

I think that right now if we have a struct with three fields where the second field is the type parameter (that we don’t know the size of) then the current code will add a dependency for accessing the first field even though we’d know the offset?

yes, that is correct, however I think that's ok

nikomatsakis (Dec 27 2018 at 12:23, on Zulip):

for one thing, the order of the fields is not actually important

nikomatsakis (Dec 27 2018 at 12:23, on Zulip):

we sort the fields so as to minimize total padding

nikomatsakis (Dec 27 2018 at 12:23, on Zulip):

when computing the layout

nikomatsakis (Dec 27 2018 at 12:23, on Zulip):

(unless the struct is #[repr(C)])

nikomatsakis (Dec 27 2018 at 12:23, on Zulip):

so that implies we would have to know the size/alignment of all fields in order to figure out the offset of any field

davidtwco (Dec 27 2018 at 12:23, on Zulip):

Ah, I see, makes sense.

nikomatsakis (Dec 27 2018 at 12:24, on Zulip):

regardless, we have enough info to improve that later

nikomatsakis (Dec 27 2018 at 12:24, on Zulip):

if we did find cases we can handle :)

nikomatsakis (Dec 27 2018 at 12:24, on Zulip):

but yeah I think there actually aren't many

nikomatsakis (Dec 27 2018 at 14:05, on Zulip):

@davidtwco pushed an initial version of the analysis for how much size reduction we can achieve

davidtwco (Dec 27 2018 at 14:05, on Zulip):

Will take a look.

nikomatsakis (Dec 27 2018 at 14:08, on Zulip):

it's slightly conservative -- it is estimating how much reduction we could get with basically zero supporting infratructure from trans. (i.e., without threading any dynamic information or otherwise getting smarter)

nikomatsakis (Dec 28 2018 at 14:08, on Zulip):

@davidtwco I don't see any new commits, just checking: did you get a chance to look into the DROP stuff here?

davidtwco (Dec 28 2018 at 14:13, on Zulip):

Not yet. Was travelling back from home yesterday and ended up not having a chance to.

davidtwco (Dec 28 2018 at 14:13, on Zulip):

I’ll be able to take a look in a little bit though.

nikomatsakis (Dec 28 2018 at 14:16, on Zulip):

OK.

nikomatsakis (Dec 28 2018 at 14:16, on Zulip):

No great hurry

nikomatsakis (Dec 28 2018 at 14:16, on Zulip):

I am looking a bit into the cross-crate question

nikomatsakis (Dec 28 2018 at 14:19, on Zulip):

actually, maybe we can do this w/o disturbing the metadata encoder -- if we are calling a fn from across crates, either it must already be monomorphic, or we must have its MIR so we can monomorphize it

nikomatsakis (Dec 28 2018 at 14:19, on Zulip):

so we could just re-analyze it

davidtwco (Dec 28 2018 at 15:16, on Zulip):

Added a commit that makes the drop cases work as expected.

davidtwco (Dec 28 2018 at 15:16, on Zulip):

I think it's perhaps a bit too naive but it worked?

nikomatsakis (Dec 28 2018 at 15:50, on Zulip):

@davidtwco I think the better way to set that up would be to move the "if it contains a type parameter" logic into the record_dependency function

nikomatsakis (Dec 28 2018 at 15:51, on Zulip):

also, I think you can use the needs_subst() helper for that

nikomatsakis (Dec 28 2018 at 15:51, on Zulip):

but otherwise, probably ok

nikomatsakis (Dec 28 2018 at 15:51, on Zulip):

hmm

nikomatsakis (Dec 28 2018 at 15:51, on Zulip):

actually, there is a helper for this we can use to get more precise

nikomatsakis (Dec 28 2018 at 15:51, on Zulip):

e.g., we know that &T never needs drop

nikomatsakis (Dec 28 2018 at 15:52, on Zulip):

ty.needs_drop(self.tcx, self.param_env)

nikomatsakis (Dec 28 2018 at 15:52, on Zulip):

let me add a test :)

davidtwco (Dec 28 2018 at 15:52, on Zulip):

I wasn't sure if needs_drop was redundant because we were already in a TerminatorKind::Drop.

nikomatsakis (Dec 28 2018 at 15:52, on Zulip):

hmm

nikomatsakis (Dec 28 2018 at 15:53, on Zulip):

you're not wrong, but you are wrong :P

nikomatsakis (Dec 28 2018 at 15:53, on Zulip):

that is, you're correct that MiR construction tries to avoid adding TerminatorKind::Drop

nikomatsakis (Dec 28 2018 at 15:53, on Zulip):

but in indirect cases it is I think still relevant

nikomatsakis (Dec 28 2018 at 15:53, on Zulip):

one sec, I'll add a few more tests to show the problem

nikomatsakis (Dec 28 2018 at 15:55, on Zulip):

or hmm it's behaving correctly ;)

nikomatsakis (Dec 28 2018 at 15:55, on Zulip):

but still doesn't feel right

davidtwco (Dec 28 2018 at 15:56, on Zulip):

I've got it building locally with .needs_drop() helper instead of the .walk().any(..) approach.

nikomatsakis (Dec 28 2018 at 15:56, on Zulip):

yeah so that seems nicer but I still feel like that code is in the wrong place

nikomatsakis (Dec 28 2018 at 15:56, on Zulip):

and I'm surprised the tests are passing

davidtwco (Dec 28 2018 at 15:57, on Zulip):

I also moved it to the record_dependency function.

nikomatsakis (Dec 28 2018 at 15:57, on Zulip):

pushed a few new tests anyway

nikomatsakis (Dec 28 2018 at 15:57, on Zulip):

ok, but locally I don't see a failure

nikomatsakis (Dec 28 2018 at 15:57, on Zulip):

which I can't quite explain

davidtwco (Dec 28 2018 at 15:57, on Zulip):

It's easy enough to adjust the check, so if your new tests keep passing with that then I'll push it.

nikomatsakis (Dec 28 2018 at 15:58, on Zulip):

ok. I expect that in the cross-crate case, we will call record_dependency, and (the way it's setup locally) that means we'll unconditionally record the substituted dependency

nikomatsakis (Dec 28 2018 at 16:00, on Zulip):

(adding a bit of logging)

davidtwco (Dec 28 2018 at 16:01, on Zulip):

Pushed the moved check/needs_drop change.

nikomatsakis (Jan 02 2019 at 19:07, on Zulip):

ok @davidtwco I think another useful thing would be to handle cross-crate edges better -- do you have time to investigate?

davidtwco (Jan 02 2019 at 19:07, on Zulip):

I've got some time, yeah.

nikomatsakis (Jan 02 2019 at 19:13, on Zulip):

so I realized that this is probably easier than I initially thought

nikomatsakis (Jan 02 2019 at 19:15, on Zulip):

currently we have this:

                if call_edge.callee.is_local() {
                    substituted_dependencies = visitors[&call_edge.callee]
                        .dependencies
                        .iter()
                        .map(|dependency| dependency.subst(tcx, call_edge.substs))
                        .collect();
                } else {
                    // FIXME: cross-crate dependencies. For now, assume that they depend
                    // on.. something.
                    substituted_dependencies = vec![Dependency {
                        span: call_edge.span,
                        kind: DependencyKind::OtherMethod(call_edge.substs),
                    }];
                }
nikomatsakis (Jan 02 2019 at 19:15, on Zulip):

but what I realized is that for any generic callee, the MIR is available to us

nikomatsakis (Jan 02 2019 at 19:16, on Zulip):

it will require some juggling though

nikomatsakis (Jan 02 2019 at 19:16, on Zulip):

right now we have this 2-phase setup where we first walk all the (local) def-ids and do the initial visitor

nikomatsakis (Jan 02 2019 at 19:16, on Zulip):

and then we walk the call edges and propagate effects around

nikomatsakis (Jan 02 2019 at 19:16, on Zulip):

I think what we would want to do is to make it possible to do the "initial visit" lazilly

nikomatsakis (Jan 02 2019 at 19:16, on Zulip):

and/or make that first pass run through a queue of def-ids

nikomatsakis (Jan 02 2019 at 19:16, on Zulip):

that we extend as we encounter callees

nikomatsakis (Jan 02 2019 at 19:17, on Zulip):

alternatively

nikomatsakis (Jan 02 2019 at 19:17, on Zulip):

hmm

nikomatsakis (Jan 02 2019 at 19:17, on Zulip):

we already invoke tcx.collect_and_partition_mono_items

nikomatsakis (Jan 02 2019 at 19:17, on Zulip):

which is basically doing this for us

nikomatsakis (Jan 02 2019 at 19:17, on Zulip):

what we could do is to invoke that, which will give us a series of MonoItem

nikomatsakis (Jan 02 2019 at 19:18, on Zulip):

we can then iterate over that list and make sure that each def-id within the list has a visitor

nikomatsakis (Jan 02 2019 at 19:18, on Zulip):

or at least each def-id meeting certain criteria, or something

nikomatsakis (Jan 02 2019 at 19:19, on Zulip):

well I really mean each InstanceDef::Item, I suspect

davidtwco (Jan 02 2019 at 19:25, on Zulip):

I could experiment with something like that.

nikomatsakis (Jan 02 2019 at 19:31, on Zulip):

ok great, let me know how it goes

nikomatsakis (Jan 03 2019 at 16:09, on Zulip):

Did you get a chance to look at this @davidtwco ?

davidtwco (Jan 03 2019 at 16:09, on Zulip):

Not yet, been responding to reviews so far today.

nikomatsakis (Jan 03 2019 at 16:19, on Zulip):

OK

nikomatsakis (Jan 04 2019 at 19:03, on Zulip):

@davidtwco I might poke at this a bit more today if you haven't

davidtwco (Jan 04 2019 at 19:03, on Zulip):

Feel free to, I'm just working on some changes for your review on another PR right now, was planning on tackling this next.

nikomatsakis (Jan 04 2019 at 19:51, on Zulip):

@davidtwco to start i'm doing a rebase and will push -f

davidtwco (Jan 04 2019 at 19:51, on Zulip):

Sounds good.

davidtwco (Jan 04 2019 at 22:12, on Zulip):

Have you started on this @nikomatsakis? Not got too long left in my day, but finished with other things and was going to take a look soon - just waiting on my build after pulling the rebase.

nikomatsakis (Jan 07 2019 at 15:49, on Zulip):

@davidtwco I did the rebase, but I didn't get any further

davidtwco (Jan 08 2019 at 09:53, on Zulip):

@nikomatsakis I've got something locally that continues to pass all our tests that also considers the DefIds from the collect_and_partition_mono_items (I had to continue to use the body_owners results too - as the mono items didn't include our local functions). I'm not sure if you have a test case that this should make pass or not. I've not done anything to make this lazy yet, not sure how I'd go about doing that.

davidtwco (Jan 08 2019 at 10:55, on Zulip):

I am observing that the test functions (eg. depend_size_alignment) aren't being returned from collect_and_partition_mono_items at the top of polymorphize_analysis where I'm calling it - or where it was being called in making the statistics.

davidtwco (Jan 08 2019 at 10:57, on Zulip):

I'm guessing that this might be a result of these functions never being used with some concrete type at all - but I think that this would affect the space savings analysis?

mw (Jan 08 2019 at 10:59, on Zulip):

(Maybe this is helpful, I haven't read the code or the whole conversation) The collector, by default, is as lazy as possible. It will only collect functions that are transitively called from something that is publicly visible (e.g. main or functions exported from the current crate).

davidtwco (Jan 08 2019 at 11:00, on Zulip):

(Maybe this is helpful, I haven't read the code or the whole conversation) The collector, by default, is as lazy as possible. It will only collect functions that are transitively called from something that is publicly visible (e.g. main or functions exported from the current crate).

I have tried making the functions public, and I'm now about to try calling them. I think what you are describing is exactly what I'm observing though, which I'm not sure we expected. Though, I might just be missing something.

davidtwco (Jan 08 2019 at 11:02, on Zulip):

Though, in some of the test files, @nikomatsakis has calls to our test functions, so perhaps they are aware of this.

davidtwco (Jan 08 2019 at 11:28, on Zulip):

Alright, now I've got it working locally without needing the body_owners call too. Still not lazy but I think this is more in the spirit of what we wanted. I've pushed it to my fork (just in case we're not happy with it) for now.

nikomatsakis (Jan 08 2019 at 16:07, on Zulip):

Though, in some of the test files, @nikomatsakis has calls to our test functions, so perhaps they are aware of this.

yes I was aware of this and I think it's fine, we just have to measure from an end-point

nikomatsakis (Jan 08 2019 at 16:07, on Zulip):

I'll take a look @davidtwco, thanks!

nikomatsakis (Jan 08 2019 at 16:16, on Zulip):

it looks right to me

nikomatsakis (Jan 08 2019 at 16:17, on Zulip):

the only thing I would say is: there is no need to invoke the fns we want to consider multiple times

nikomatsakis (Jan 08 2019 at 16:19, on Zulip):

I don't think I intended to add -Zpolymorphize-dump to each of the tests though..

nikomatsakis (Jan 08 2019 at 16:19, on Zulip):

...we could make dump the default, and add a flag to add all local methods for analysis

nikomatsakis (Jan 08 2019 at 16:19, on Zulip):

or we can just invoke them

nikomatsakis (Jan 08 2019 at 16:19, on Zulip):

the latter seems fine I guess

davidtwco (Jan 08 2019 at 16:23, on Zulip):

I didn’t think it was required to invoke each multiple times, I just wanted it to be consistent with the existing test that had invocations.

Unless we want to merge the body owners ids and mono item ids, we’ll need to invoke each function with this change.

nikomatsakis (Jan 08 2019 at 16:24, on Zulip):

right. apart from testing, I can't think of a reason to merge that

nikomatsakis (Jan 08 2019 at 16:25, on Zulip):

so I guess might as well leave it how you have it

nikomatsakis (Jan 08 2019 at 16:25, on Zulip):

it seems like we're close to the point where we can get some data now

nikomatsakis (Jan 08 2019 at 16:25, on Zulip):

it occurs to me it might be nice to have a mode that dumps out more detail about the duplicates

nikomatsakis (Jan 08 2019 at 16:25, on Zulip):

as a sanity check

nikomatsakis (Jan 08 2019 at 16:26, on Zulip):

maybe repurpose the "dump" flag

nikomatsakis (Jan 08 2019 at 16:27, on Zulip):

er

nikomatsakis (Jan 08 2019 at 16:27, on Zulip):

wait

nikomatsakis (Jan 08 2019 at 16:27, on Zulip):

the dump flag is the one that shows the errors?

nikomatsakis (Jan 08 2019 at 16:27, on Zulip):

we should probably keep that

nikomatsakis (Jan 08 2019 at 16:27, on Zulip):

maybe just emit the duplicates as info! level logging, then we can turn it on with RUST_LOG

nikomatsakis (Jan 08 2019 at 16:27, on Zulip):

I'm guessing there are still dependencies we are not considering

nikomatsakis (Jan 08 2019 at 16:27, on Zulip):

not 100% sure what those are

nikomatsakis (Jan 08 2019 at 16:28, on Zulip):

I'll have to go over the list again

nikomatsakis (Jan 08 2019 at 16:28, on Zulip):

simple example: we consider fields, I think, but if you have foo.bar[x], then the type of foo.bar had better have a known size

nikomatsakis (Jan 08 2019 at 16:28, on Zulip):

don't think we check for that

davidtwco (Jan 08 2019 at 16:33, on Zulip):

I'll look into adding that in a bit.

davidtwco (Jan 08 2019 at 16:33, on Zulip):

Will push my latest change to your branch too.

davidtwco (Jan 08 2019 at 18:35, on Zulip):

@nikomatsakis I'm struggling to come up with an example where a x[i] (for some x: T) ends up as a ProjectionElem::Index(..) and not a call to std::ops::Index::index, any ideas?

nikomatsakis (Jan 08 2019 at 19:13, on Zulip):

@davidtwco indexing into a &[T] slice..maybe something like this?

fn dependency_because_index_depends_on_T_sized<T>(parameters: &[T]) -> &T {
    //~^ ERROR some polymorphic dependencies found
    &parameters[0]
}
nikomatsakis (Jan 08 2019 at 19:14, on Zulip):

the name is slightly wrong

nikomatsakis (Jan 08 2019 at 19:14, on Zulip):

even though we know that T: Sized, we still don't know how big it is -- or maybe you want &parameters[1], since..strictly speaking..&parameters[0] doesn't depend

davidtwco (Jan 08 2019 at 21:31, on Zulip):

Pushed a commit with that check, still considers x[0] a dependency for the moment.

nikomatsakis (Jan 09 2019 at 14:56, on Zulip):

seems ok

nikomatsakis (Jan 09 2019 at 14:57, on Zulip):

nice!

nikomatsakis (Jan 09 2019 at 14:57, on Zulip):

I'll try to do another pass for missing things, but it seems like it'd be interesting to also start gathering some data now

davidtwco (Jan 09 2019 at 14:58, on Zulip):

Running -Z polymorphize on actual crates?

nikomatsakis (Jan 09 2019 at 14:58, on Zulip):

yeah, although it'll probably be so horrificially slow

nikomatsakis (Jan 09 2019 at 14:58, on Zulip):

maybe we should look at opimizing the propagation a bit

nikomatsakis (Jan 09 2019 at 14:59, on Zulip):

otoh who cares

nikomatsakis (Jan 09 2019 at 14:59, on Zulip):

I guess it depends how slow

davidtwco (Jan 09 2019 at 15:00, on Zulip):

I guess the only downside to speeding it up now is that if we find that it isn't worth doing polymorphization then that would have been wasted effort?

nikomatsakis (Jan 09 2019 at 16:03, on Zulip):

I guess

nikomatsakis (Jan 09 2019 at 16:03, on Zulip):

I mean it seems worth trying it on a few smaller tests to start

nikomatsakis (Jan 09 2019 at 16:03, on Zulip):

before we invest too much effort in optimizing

nikomatsakis (Jan 09 2019 at 16:03, on Zulip):

not sure what a smaller test would be

nikomatsakis (Jan 09 2019 at 16:03, on Zulip):

e.g. I'm curious to measure something like ripgrep

nikomatsakis (Jan 09 2019 at 16:03, on Zulip):

I guess to do this properly we need to basically apply it to the transitive set of crates
a

nikomatsakis (Jan 09 2019 at 16:03, on Zulip):

and take the data from each one and collect

nikomatsakis (Jan 09 2019 at 16:04, on Zulip):

that is, at each crate, we'll have various monomorphizations that we print out (and various things that were not locally used and hence which (may) be instantiated by clients)

nikomatsakis (Jan 09 2019 at 16:15, on Zulip):

I guess we could look to https://perf.rust-lang.org/ to get some ideas for benchmarks. Other thoughts:

nikomatsakis (Jan 09 2019 at 16:16, on Zulip):

maybe something crazy like https://github.com/gluon-lang/gluon

nikomatsakis (Jan 09 2019 at 16:16, on Zulip):

or https://github.com/dagit/rust-prolog

nikomatsakis (Jan 09 2019 at 22:54, on Zulip):

welp my attempt to run on cw immediately panicked :)

nikomatsakis (Jan 09 2019 at 22:54, on Zulip):

something about invoking Instance::resolve failed

davidtwco (Jan 09 2019 at 22:55, on Zulip):

:frown:

nikomatsakis (Jan 09 2019 at 22:56, on Zulip):

I think I know the problem tho

nikomatsakis (Jan 09 2019 at 22:57, on Zulip):

in particular we need to normalize after substituting here:

                    substituted_dependencies = visitor.dependencies.iter()
                        .map(|dependency| dependency.subst(tcx, call_edge.substs))
                        .collect();
nikomatsakis (Jan 09 2019 at 22:58, on Zulip):

we could probably do the whole "trans thing" and erase regions too

nikomatsakis (Jan 09 2019 at 22:58, on Zulip):

I wonder if there's a helper for this

nikomatsakis (Jan 09 2019 at 23:03, on Zulip):

yes

nikomatsakis (Jan 09 2019 at 23:23, on Zulip):

ye

nikomatsakis (Jan 09 2019 at 23:23, on Zulip):

ye

nikomatsakis (Jan 09 2019 at 23:23, on Zulip):
athena. RUST_BACKTRACE=1 cargo +rust-7-stage2 rustc -- -Zpolymorphize
   Compiling cw v0.3.0 (/home/nmatsakis/versioned/regr/cw)
note: Monomorphized items: 483

note: Monomorphized size : 10664

note: New total items    : 475 ( 98%)

note: New estimated size : 10579 ( 99%)
nikomatsakis (Jan 09 2019 at 23:23, on Zulip):

not a complete measurement though

davidtwco (Jan 09 2019 at 23:24, on Zulip):

That's not a particularly large decrease.

nikomatsakis (Jan 09 2019 at 23:24, on Zulip):

nope

nikomatsakis (Jan 09 2019 at 23:24, on Zulip):

not that I necessarily expected it would be for that particular test

nikomatsakis (Jan 09 2019 at 23:24, on Zulip):

not sure if others will do better

davidtwco (Jan 09 2019 at 23:33, on Zulip):

how was the performance running the analysis on a real crate?

nikomatsakis (Jan 10 2019 at 14:17, on Zulip):

seemed fine

nikomatsakis (Jan 10 2019 at 14:18, on Zulip):

@davidtwco I was thinking of two things we should do:

So for example, moving a value of type T is possible in level 1 but not level 0. Similarly indexing a slice of type &[T] is possible in level 1 but not level 0. However, indexing into a struct like (u32, T) is not possible in either level (because we'd have to do complex calculations to figure out the offset.

nikomatsakis (Jan 10 2019 at 14:18, on Zulip):

we might then add more levels in the future

nikomatsakis (Jan 10 2019 at 14:19, on Zulip):

also, we should add some "sanity check" tests for cross-crate -- e.g., I'd like to test the following function:

fn main() {
    let x: Vec<u8> = vec![1, 2, 3];
    x.len();

    let x: Vec<u32> = vec![1, 2, 3];
    x.len();
}

I think x.len() can get collapsed, but worth testing.

davidtwco (Jan 10 2019 at 14:20, on Zulip):

Alright, I can probably find some time to do those later.

nikomatsakis (Jan 10 2019 at 14:24, on Zulip):

Cool =)

davidtwco (Jan 10 2019 at 21:13, on Zulip):

Pushed the above (except the cross-crate sanity check)

nikomatsakis (Jan 10 2019 at 21:53, on Zulip):

cool

nikomatsakis (Jan 10 2019 at 22:28, on Zulip):

@davidtwco I think that this is a bit too permissive:

                match SizeSkeleton::compute(ty, self.tcx, self.param_env) {
                    Ok(SizeSkeleton::Known(_)) => {
                        debug!("record_dependency: known size, skipping");
                        false
                    }
                    _ if level > 0 && !kind.is_offset_of() =>  {
                        debug!("record_dependency: unknown size, skipping as level above zero");
                        false
                    }
                    _ => {
                        debug!("record_dependency: recording dependency");
                        self.push_dependency_if_new(span, kind)
                    }
                }

specifically, I think we want to check for cases where the type we need the size of is exactly a parameter

nikomatsakis (Jan 10 2019 at 22:29, on Zulip):

i.e., the type of A

nikomatsakis (Jan 10 2019 at 22:29, on Zulip):

in contrast to something that includes A, like (A, u32)

nikomatsakis (Jan 10 2019 at 22:29, on Zulip):

computing the size of the derived type is more complex

nikomatsakis (Jan 10 2019 at 22:29, on Zulip):

I can make an example perhaps

davidtwco (Jan 10 2019 at 22:30, on Zulip):

I understand what you mean, I’ll look at doing that.

nikomatsakis (Jan 10 2019 at 22:30, on Zulip):

actually the test you have is confusing me a bit

nikomatsakis (Jan 10 2019 at 22:31, on Zulip):

but also I maybe realized a complication

davidtwco (Jan 10 2019 at 22:31, on Zulip):

That should just be checking that ty is a ty::TyKind::TyParam(..).

nikomatsakis (Jan 10 2019 at 22:31, on Zulip):

yes, right, I meant that polymorphize-level1.rs is confusing me a bit

nikomatsakis (Jan 10 2019 at 22:31, on Zulip):

in that I don't see a test that just moves a value of type T

nikomatsakis (Jan 10 2019 at 22:31, on Zulip):

but I sort of expected to

nikomatsakis (Jan 10 2019 at 22:32, on Zulip):

but we also have one other wrinkle to be careful of, one sec..

davidtwco (Jan 10 2019 at 22:32, on Zulip):

Not knowing we only wanted a lone T, I just made a test for each kind of dependency.

davidtwco (Jan 10 2019 at 22:32, on Zulip):

Since it was the function that registered them and matched on the kind that changed.

nikomatsakis (Jan 10 2019 at 22:35, on Zulip):

one complication I was thinking:

at least in level 1, when you invoke a function foo::<T1, T2> -- then each of the types T1 and T2 must have a known size

nikomatsakis (Jan 10 2019 at 22:36, on Zulip):

as an example:

// just needs to know the size of `T`, ok in level 1
fn foo<T>(t: Box<T>) -> T { *t }

fn bar<T>(a: T) {
    // this call winds up requiring us to compute size of (T, u32)`
    foo::<(T, T)>(Box::new((a, 22)))
}
davidtwco (Jan 10 2019 at 22:39, on Zulip):

What is the purpose of the levels?

nikomatsakis (Jan 11 2019 at 10:33, on Zulip):

@davidtwco the idea is this:

nikomatsakis (Jan 11 2019 at 10:33, on Zulip):

(this way we could try to get an idea if the runtime work is worthwhile)

davidtwco (Jan 11 2019 at 10:34, on Zulip):

I see.

davidtwco (Jan 14 2019 at 10:03, on Zulip):

Pushed a change that should make that check less permissive.

nikomatsakis (Jan 16 2019 at 14:13, on Zulip):

@davidtwco cool, let me check tha tout

davidtwco (Jan 27 2019 at 10:48, on Zulip):

@nikomatsakis what’s next here?

nikomatsakis (Jan 28 2019 at 20:44, on Zulip):

@davidtwco sigh, I've been wondering the same thing.

nikomatsakis (Jan 28 2019 at 20:44, on Zulip):

I guess we need to try it out on more crates

nikomatsakis (Jan 31 2019 at 14:27, on Zulip):

Ugh. =) @davidtwco sorry been paralyzed here =)

davidtwco (Jan 31 2019 at 14:27, on Zulip):

That's all good. I'm sure the All Hands planning is keeping you busy.

Last update: Nov 22 2019 at 04:35UTC