Stream: wg-traits

Topic: impl-trait-in-bindings


Alexander Regueiro (Nov 07 2018 at 22:51, on Zulip):

@nikomatsakis any more ideas about fixing impl Trait in bindings before I get going on it?

Alexander Regueiro (Nov 07 2018 at 22:51, on Zulip):

based off your previous advice

Alexander Regueiro (Nov 08 2018 at 03:04, on Zulip):

@nikomatsakis @scalexm Hmm. How do I get the predicates for an impl Trait ty::Ty during type checking? (the TyKind contains a DefId and Substs, but now sure how that helps directly.) predicate_for_trait_def seems almost right, but I guess that only works if the DefId passed in is for a trait, not an impl Trait type?

Alexander Regueiro (Nov 08 2018 at 03:18, on Zulip):

also, the coercion routine coerce_unsized is slightly confusing me... is the aim to create the most granular obligations possible? and is this needed for my corresponding approach for coercing to opaque types?

Alexander Regueiro (Nov 08 2018 at 03:18, on Zulip):

I think if I figure this out, I can continue making good progress.

Alexander Regueiro (Nov 08 2018 at 03:18, on Zulip):

already made some.

scalexm (Nov 08 2018 at 10:31, on Zulip):

@Alexander Regueiro I'm not sure to understand what you're asking for

scalexm (Nov 08 2018 at 10:32, on Zulip):

if you want the list of generic predicates on the impl Trait type, you can just call tcx.predicates_of(def_id)

Alexander Regueiro (Nov 08 2018 at 16:46, on Zulip):

@scalexm sure, but that doesn’t take into account the Substs no?

scalexm (Nov 08 2018 at 16:46, on Zulip):

@Alexander Regueiro you can do tcx.predicates_of(def_id).instantiate(substs) or something

scalexm (Nov 08 2018 at 16:47, on Zulip):

and then if you want a PredicateObligations in the end you can use traits::predicates_for_generics

Alexander Regueiro (Nov 08 2018 at 17:17, on Zulip):

@scalexm aha, perfect. thank you.

Alexander Regueiro (Nov 08 2018 at 17:19, on Zulip):

@scalexm there's no recursive procedure involved here, like there is with coerce_unsize, right?

scalexm (Nov 08 2018 at 17:30, on Zulip):

@Alexander Regueiro mmh I don't know how coerce_unsize works

Alexander Regueiro (Nov 08 2018 at 17:33, on Zulip):

@scalexm no worries. one final thing (for now!): what is body_id for an Obligation (some sort of context I guess?), and why is it only relevant if the predicate is a trait-ref?

Alexander Regueiro (Nov 08 2018 at 17:39, on Zulip):

@nikomatsakis you around by chance?

Alexander Regueiro (Nov 08 2018 at 17:46, on Zulip):

I'm wondering what this LUB relationship checking is for

scalexm (Nov 08 2018 at 17:51, on Zulip):

@Alexander Regueiro according to the doc it is needed for regions checking but I don't know much more =)

scalexm (Nov 08 2018 at 17:52, on Zulip):

but if you work with opaque types, I guess this is just the NodeId of the function defining your opaque type

Alexander Regueiro (Nov 08 2018 at 17:53, on Zulip):

@scalexm hmm... not where the opaque type is used?

scalexm (Nov 08 2018 at 17:58, on Zulip):

@Alexander Regueiro well I was guessing that by reading the code in typeck_tables_of

Alexander Regueiro (Nov 08 2018 at 18:09, on Zulip):

hmm

Alexander Regueiro (Nov 08 2018 at 18:10, on Zulip):

@scalexm incidentally, there's an instantiate_bounds fn too, which does something similar to what you said above, but takes a span (which I wouldn't know how to provide)

Alexander Regueiro (Nov 08 2018 at 18:11, on Zulip):

ah

Alexander Regueiro (Nov 08 2018 at 18:11, on Zulip):

maybe self.cause.span

Alexander Regueiro (Nov 08 2018 at 19:22, on Zulip):

ugh, factoring out the reborrow code into its own function is creating all sorts of lifetime mayhem

Alexander Regueiro (Nov 08 2018 at 19:27, on Zulip):

I think it's an NLL thing

Alexander Regueiro (Nov 08 2018 at 19:27, on Zulip):

wish I could turn it on in the compiler :-P

Alexander Regueiro (Nov 08 2018 at 19:37, on Zulip):

(deleted)

Alexander Regueiro (Nov 08 2018 at 19:59, on Zulip):

okay, making headway at last!

Alexander Regueiro (Nov 08 2018 at 20:09, on Zulip):

@nikomatsakis @scalexm do I still need to instantiate_opaque_types_from_value in typeck_tables_of? if not, I need to modify the type somewhere else.

scalexm (Nov 08 2018 at 20:31, on Zulip):

@Alexander Regueiro I think niko might be able to produce an answer more easily, I'm not very familiar with this code :/

Alexander Regueiro (Nov 08 2018 at 21:00, on Zulip):

@scalexm no worries

Alexander Regueiro (Nov 08 2018 at 21:01, on Zulip):

Hopefully he’ll be around later

Alexander Regueiro (Nov 09 2018 at 01:18, on Zulip):

@nikomatsakis in fact, typeck_tables isn't the only places; there are others. but the question remains: do I need to call instantiate_opaque_types_from_value for types of or within bindings, under this new approach?

Alexander Regueiro (Nov 09 2018 at 01:28, on Zulip):

@nikomatsakis it seems kind of incompatible with the coercion-based approach. but maybe I'm missing something?

Alexander Regueiro (Nov 09 2018 at 01:30, on Zulip):

if the coercion routine (coerce_hidden) itself calls instantiate_opaque_types_from_value, then it needs to know the parent fn ID, which is kind of awkward, and seems hacky. (and I don't even know what happens in the case the existential type is at the top level, i.e. no parent id.)

Alexander Regueiro (Nov 09 2018 at 02:31, on Zulip):

@nikomatsakis you can see my attempt so far at https://github.com/rust-lang/rust/pull/55807

Alexander Regueiro (Nov 10 2018 at 17:34, on Zulip):

Made some good progress now, by the way. Will push soon. :-)

Alexander Regueiro (Nov 10 2018 at 17:34, on Zulip):

Sorry if I was pinging you too much last week. I forgot you were at the conference once or twice, and are a busy man. My bad.

Alexander Regueiro (Nov 13 2018 at 19:33, on Zulip):

@nikomatsakis I'm going to go out for the day, so last chance to talk, FYI

Alexander Regueiro (Nov 13 2018 at 19:36, on Zulip):

otherwise just feel free to leave me notes ;-)

Alexander Regueiro (Nov 13 2018 at 19:36, on Zulip):

I could share the MIR with you now if you like though

Alexander Regueiro (Nov 13 2018 at 19:46, on Zulip):

mir_dump.zip

Alexander Regueiro (Nov 13 2018 at 19:46, on Zulip):

there you go, in fact ^

Alexander Regueiro (Nov 13 2018 at 19:47, on Zulip):

impllet_a.rs -- the test file

Alexander Regueiro (Nov 13 2018 at 19:48, on Zulip):

codegen error output:

error: internal compiler error: librustc_codegen_llvm/mir/block.rs:753: codegen_argument: OperandRef(Immediate((i8**:  %4 = alloca i8*, align 8)) @ TyLayout { ty: &dyn std::ops::Fn() -> i32 + std::panic::RefUnwindSafe + std::marker::Sync, details: LayoutDetails { variants: Single { index: 0 }, fields: Arbitrary { offsets: [Size { raw: 0 }, Size { raw: 8 }], memory_index: [0, 1] }, abi: ScalarPair(Scalar { value: Pointer, valid_range: 1..=18446744073709551615 }, Scalar { value: Pointer, valid_range: 1..=18446744073709551615 }), align: Align { abi_pow2: 3, pref_pow2: 3 }, size: Size { raw: 16 } } }) invalid for pair argument

thread 'main' panicked at 'encountered error with `-Z treat_err_as_bug', librustc_errors/lib.rs:500:13
stack backtrace:
   0: std::sys::unix::backtrace::tracing::imp::unwind_backtrace
   1: std::sys_common::backtrace::print
   2: std::panicking::default_hook::{{closure}}
   3: std::panicking::default_hook
   4: rustc::util::common::panic_hook
   5: std::panicking::rust_panic_with_hook
   6: std::panicking::begin_panic
   7: rustc_errors::Handler::emit_db
   8: rustc_errors::Handler::bug
   9: rustc::util::bug::opt_span_bug_fmt::{{closure}}
  10: rustc::ty::context::tls::with_opt::{{closure}}
  11: rustc::ty::context::tls::with_context_opt
  12: rustc::ty::context::tls::with_opt
  13: rustc::util::bug::opt_span_bug_fmt
  14: rustc::util::bug::bug_fmt
  15: rustc_codegen_llvm::mir::block::<impl rustc_codegen_llvm::mir::FunctionCx<'a, 'll, 'tcx>>::codegen_argument
  16: rustc_codegen_llvm::mir::block::<impl rustc_codegen_llvm::mir::FunctionCx<'a, 'll, 'tcx>>::codegen_terminator
  17: rustc_codegen_llvm::mir::codegen_mir
  18: rustc_codegen_llvm::base::codegen_instance
  19: rustc_codegen_llvm::mono_item::MonoItemExt::define
  20: rustc_codegen_llvm::base::compile_codegen_unit::module_codegen
...
Alexander Regueiro (Nov 13 2018 at 19:48, on Zulip):

bye for now!

Alexander Regueiro (Nov 15 2018 at 19:24, on Zulip):

@nikomatsakis regarding your recent notes (thanks for those), was there any reason you suggested to use instantiate_opaque_types in the nll type checking part rather than regular type checking?

Alexander Regueiro (Nov 15 2018 at 19:24, on Zulip):

I thought it needed to be used somewhere, but not sure why there

nikomatsakis (Nov 19 2018 at 19:52, on Zulip):

Heh @Alexander Regueiro I'm not sure I'll have to re-review

Alexander Regueiro (Nov 19 2018 at 20:06, on Zulip):

@nikomatsakis no worries.

Alexander Regueiro (Nov 19 2018 at 20:07, on Zulip):

your existing advice is helpful... though I haven't turned my attention properly to that PR yet :-)

Alexander Regueiro (Nov 22 2018 at 16:17, on Zulip):

@nikomatsakis let me know when you get a chance to look at that one little point again...

Alexander Regueiro (Nov 23 2018 at 15:56, on Zulip):

@nikomatsakis it's literally just the thing about where to do the type instantiation for opaque types (in normal type checking or NLL)

Alexander Regueiro (Nov 29 2018 at 22:21, on Zulip):

@nikomatsakis I presume you didn't forget about the above...

nikomatsakis (Nov 30 2018 at 21:00, on Zulip):

ok circling back to this now :)

nikomatsakis (Nov 30 2018 at 21:00, on Zulip):

sorry

nikomatsakis (Nov 30 2018 at 21:00, on Zulip):

is the remaining question to be found in https://github.com/rust-lang/rust/pull/55807 ?

Alexander Regueiro (Nov 30 2018 at 21:39, on Zulip):

Hi

Alexander Regueiro (Nov 30 2018 at 21:39, on Zulip):

@nikomatsakis mainly about this comment https://github.com/rust-lang/rust/pull/55807#discussion_r233631877

Alexander Regueiro (Nov 30 2018 at 21:40, on Zulip):

and whether it would actually be better to do instantiate_opaque_types in normal type checking, rather than NLL type checking as you suggested?

Alexander Regueiro (Nov 30 2018 at 21:40, on Zulip):

also, I'm slightly struggling to see how opaque type instantiation and coercion fit together. maybe this relates to your last review comment.

nikomatsakis (Nov 30 2018 at 21:56, on Zulip):

and whether it would actually be better to do instantiate_opaque_types in normal type checking, rather than NLL type checking as you suggested?

Ah. I think we're going to have to do it in both places probably, ultimately

nikomatsakis (Nov 30 2018 at 21:56, on Zulip):

but let me re-read whatI wrote

Alexander Regueiro (Nov 30 2018 at 21:59, on Zulip):

sure :-)

nikomatsakis (Nov 30 2018 at 22:11, on Zulip):

(I'm reading a bit into the way that impl Trait works now, it's changed as a result of @Oli's excellent refactorings...)

Alexander Regueiro (Nov 30 2018 at 22:13, on Zulip):

aha

Alexander Regueiro (Nov 30 2018 at 22:13, on Zulip):

how recently did it change? :-)

nikomatsakis (Nov 30 2018 at 22:14, on Zulip):

not that recently

nikomatsakis (Nov 30 2018 at 22:14, on Zulip):

so when I say "defining use" does that mean anything to you? :)

nikomatsakis (Nov 30 2018 at 22:15, on Zulip):

similarly, do you grok what this code is trying to do?

nikomatsakis (Nov 30 2018 at 22:15, on Zulip):

e.g., the distinction between these two different uses of the abtract type Foo?

nikomatsakis (Nov 30 2018 at 22:21, on Zulip):

(trying to calibrate how much you've read into that code)

Alexander Regueiro (Nov 30 2018 at 22:23, on Zulip):

intuitively, "defining use" makes sense, I think

Alexander Regueiro (Nov 30 2018 at 22:23, on Zulip):

let me see...

Alexander Regueiro (Nov 30 2018 at 22:25, on Zulip):

ah yes

Alexander Regueiro (Nov 30 2018 at 22:26, on Zulip):

so... I'm guessing we want to instantiate opaque types in the case of defining uses, but coerce in the other case?

Alexander Regueiro (Nov 30 2018 at 22:27, on Zulip):

^ @nikomatsakis

nikomatsakis (Nov 30 2018 at 22:41, on Zulip):

Right. But here the "defining use" definition gets more complex than before; we used to only consider the parent_def_id

nikomatsakis (Nov 30 2018 at 22:41, on Zulip):

but we now want to be at the granularity of a specific let statement

Alexander Regueiro (Nov 30 2018 at 22:42, on Zulip):

yeah, I was wondering about that... sounds like you're right.

nikomatsakis (Nov 30 2018 at 22:42, on Zulip):

we might want some kind of separate fn that says

nikomatsakis (Nov 30 2018 at 22:42, on Zulip):

er

nikomatsakis (Nov 30 2018 at 22:42, on Zulip):

we might want to refactor it sort of so that it is given a kind of "context"

Alexander Regueiro (Nov 30 2018 at 22:42, on Zulip):

let statement or assignment could be the defining use now, right?

nikomatsakis (Nov 30 2018 at 22:43, on Zulip):

yes but

Alexander Regueiro (Nov 30 2018 at 22:43, on Zulip):

hmm, how do you mean exactly?

nikomatsakis (Nov 30 2018 at 22:44, on Zulip):

sorry I'm getting interrupted here :)

nikomatsakis (Nov 30 2018 at 22:44, on Zulip):

not being very clear

nikomatsakis (Nov 30 2018 at 22:44, on Zulip):

I'm also not sure what I would do :P

nikomatsakis (Nov 30 2018 at 22:44, on Zulip):

I can see a few ways of approaching it

nikomatsakis (Nov 30 2018 at 22:45, on Zulip):

but in any case, what we want is that:

when you have a let, we invoke instantiate_opaque_types on the type annotation. This is considered the 'defining use' for any impl Trait that appeared within that let, just as the "return type" is the "defining use" for impl Trait in that position

nikomatsakis (Nov 30 2018 at 22:45, on Zulip):

other opaque types are left alone :)

nikomatsakis (Nov 30 2018 at 22:46, on Zulip):

thinking more about it, I think we do not have to modify the coercion.rs code

nikomatsakis (Nov 30 2018 at 22:46, on Zulip):

once we get the instantiate_opaque_type call correct, you will basically wind up with a set of types that (may) include inference variables

nikomatsakis (Nov 30 2018 at 22:46, on Zulip):

these are what we coerce to

nikomatsakis (Nov 30 2018 at 22:47, on Zulip):

then there is one add'l coercion atop that (maybe not treated as a "adjustment", maybe added in some other way, but I think it should appear in the MIR, where this context is gone) that "casts" to the opaque types

Alexander Regueiro (Nov 30 2018 at 22:47, on Zulip):

right

Alexander Regueiro (Nov 30 2018 at 22:47, on Zulip):

makes sense

Alexander Regueiro (Nov 30 2018 at 22:48, on Zulip):

but the code I wrote for coercion to opaque types more or less makes sense, I think... no?

Alexander Regueiro (Nov 30 2018 at 22:48, on Zulip):

that should stay?

nikomatsakis (Nov 30 2018 at 22:49, on Zulip):

I think you don't want it

nikomatsakis (Nov 30 2018 at 22:49, on Zulip):

or need it

nikomatsakis (Nov 30 2018 at 22:49, on Zulip):

let me go look again though

nikomatsakis (Nov 30 2018 at 22:49, on Zulip):

that was the code in coercion.rs?

nikomatsakis (Nov 30 2018 at 22:50, on Zulip):

we might want it somewhere else

Alexander Regueiro (Nov 30 2018 at 22:50, on Zulip):

well, I added a MIR "hiding" operation

nikomatsakis (Nov 30 2018 at 22:50, on Zulip):

this "cast" in the MIR I'm talking about; presumably it has the form of a kind of "user-assert ty". That is, it has to have the type that the user gave (with impl Trait) and it knows it is the defining use for those types (so it can invoke the instantiate again)

nikomatsakis (Nov 30 2018 at 22:51, on Zulip):

ok I see that code is probably basically right

nikomatsakis (Nov 30 2018 at 22:51, on Zulip):

though I've forgotten just how it looked

Alexander Regueiro (Nov 30 2018 at 22:51, on Zulip):

@nikomatsakis but the coerce_hidden code is probably what you'd remove, or what?

Alexander Regueiro (Nov 30 2018 at 22:53, on Zulip):

anyway I'm trying to understand why you're talking of two forms of coercion, and whether coerce_hidden corresponds to either.

Alexander Regueiro (Nov 30 2018 at 22:55, on Zulip):

incidentally, I guess we only call instantiate_opaque_types for locals if there is at least one opaque types somewhere within the local's type.

nikomatsakis (Nov 30 2018 at 22:56, on Zulip):

I don't really think we want a coercion per se

nikomatsakis (Nov 30 2018 at 22:57, on Zulip):

i.e., not the compiler code for cercions

nikomatsakis (Nov 30 2018 at 22:57, on Zulip):

but there is a type conversion that is happening

nikomatsakis (Nov 30 2018 at 22:57, on Zulip):

if you have:

let x: T = <expr>;

where T involves some impl Trait

nikomatsakis (Nov 30 2018 at 22:58, on Zulip):

you wind up with two variants of T, one that has inference variables for impl Trait, and one that has opaque values

nikomatsakis (Nov 30 2018 at 22:58, on Zulip):

let's call them T_infer and T_opaque`

nikomatsakis (Nov 30 2018 at 22:58, on Zulip):

we want to coerce typeof(<expr>) to T_infer

nikomatsakis (Nov 30 2018 at 22:58, on Zulip):

but use T_opaque as the type for the pattern x

nikomatsakis (Nov 30 2018 at 22:59, on Zulip):

then we need in the HAIR/MIR to reflect that "link" somehow to

nikomatsakis (Nov 30 2018 at 22:59, on Zulip):

I think originally I did recommend modifying or at least looking at coercion.rs, but now I think that's not quite right

nikomatsakis (Nov 30 2018 at 23:00, on Zulip):

where did I leave the original set of mentoring instructions, @Alexander Regueiro ?

nikomatsakis (Nov 30 2018 at 23:00, on Zulip):

(we had an issue for this?)

Alexander Regueiro (Nov 30 2018 at 23:03, on Zulip):

@nikomatsakis hmm, on the issue that eddyb filed I think?

Alexander Regueiro (Nov 30 2018 at 23:03, on Zulip):

or the subsequent quickfix PR you submitted?

Alexander Regueiro (Nov 30 2018 at 23:03, on Zulip):

I forget

nikomatsakis (Nov 30 2018 at 23:09, on Zulip):

https://github.com/rust-lang/rust/issues/54600

nikomatsakis (Nov 30 2018 at 23:09, on Zulip):

I have to run now but now that I've refreshed my memory, i'll re-skim those and try to write some updated ones :)

Alexander Regueiro (Nov 30 2018 at 23:09, on Zulip):

@nikomatsakis found it: https://github.com/rust-lang/rust/issues/54600#issuecomment-433049875

Alexander Regueiro (Nov 30 2018 at 23:09, on Zulip):

hah

Alexander Regueiro (Nov 30 2018 at 23:10, on Zulip):

@nikomatsakis okay, would appreciate that, thanks. if I get started in the meanwhile, how do you recommend I store both the instantiated and original opaque type side-by-side?

nikomatsakis (Nov 30 2018 at 23:10, on Zulip):

I think the key steps are going to be:

that's about it

nikomatsakis (Nov 30 2018 at 23:10, on Zulip):

I don't know that you have to store them side-by-side per se

nikomatsakis (Nov 30 2018 at 23:11, on Zulip):

usually in the writeback phase we record the type we inferred for impl trait

nikomatsakis (Nov 30 2018 at 23:11, on Zulip):

so that we can reinstantiate it with that type

Alexander Regueiro (Nov 30 2018 at 23:11, on Zulip):

that knows it has to instantiate...what?

Alexander Regueiro (Nov 30 2018 at 23:11, on Zulip):

right, I was wondering whether writeback handled it all...

Alexander Regueiro (Nov 30 2018 at 23:11, on Zulip):

fair enough

Alexander Regueiro (Dec 13 2018 at 19:31, on Zulip):

now... I don't think I need more from you on the impl-trait-in-bindings stuff. I'm only just getting back to working on that now that I've gotten these two PRs out of the way heh.

nikomatsakis (Dec 13 2018 at 19:32, on Zulip):

ok :) I can't remember just where we left that

Alexander Regueiro (Dec 13 2018 at 19:32, on Zulip):

you've left me some good pointers. though I realise you haven't fully made up your mind on all points.

nikomatsakis (Dec 13 2018 at 19:32, on Zulip):

yeah, I think I was busily revisiting some of what I had written initially iirc

Alexander Regueiro (Dec 13 2018 at 19:32, on Zulip):

some decisions will require experimentations I think

Alexander Regueiro (Dec 13 2018 at 19:32, on Zulip):

yeah

Alexander Regueiro (Dec 13 2018 at 19:32, on Zulip):

and you weren't quite sure on the nature of coercions needed?

nikomatsakis (Dec 13 2018 at 19:33, on Zulip):

ok yes so basically I was saying: I don't think we want to do the "coercion" as part of coerce.rs

Alexander Regueiro (Dec 13 2018 at 19:33, on Zulip):

and how distinction should be made between defining and non-defining uses

Alexander Regueiro (Dec 13 2018 at 19:33, on Zulip):

in this more complex case

Alexander Regueiro (Dec 13 2018 at 19:33, on Zulip):

(it may not be as simple as just a let statement... or will it?)

Alexander Regueiro (Dec 13 2018 at 19:33, on Zulip):

mhm

nikomatsakis (Dec 13 2018 at 19:33, on Zulip):

this is somewhat related actually to some of the stuff that we've been wrestling with in #wg-nll about what precisely let pat: T = .. actually means

nikomatsakis (Dec 13 2018 at 19:34, on Zulip):

in particular, is the type T a kind of supertype for the right-hand side... or is it specifying the type of the binding in pattern ... or both

nikomatsakis (Dec 13 2018 at 19:34, on Zulip):

e.g., let _: T = ... -- here, there is no binding in pattern, so it must be acting as a supertype for the right-hand side

Alexander Regueiro (Dec 13 2018 at 19:35, on Zulip):

aha

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

but if you have let x: &'static u32 = ..., would it be ok for the compiler to assign x a type like &'a u32, since &'static u32 <: &'a u32?

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

probably not, that's sort of surprising

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

anyway, it seems related to me, but maybe it won't come up that much

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

I guess mainly it seems like the "defining use" is the "type(rhs) <: T" test

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

but the type that gets assigned to the bindings in pat uses the "opaque types"

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

so you can see the split again arising

Alexander Regueiro (Dec 13 2018 at 19:36, on Zulip):

well, the opposite sounds fine (i.e. promotion), in certain cases, but that... not so much

Alexander Regueiro (Dec 13 2018 at 19:37, on Zulip):

okay, that seems sensible for a defining use condition

Alexander Regueiro (Dec 13 2018 at 19:37, on Zulip):

so it's like a try-coerce or something, but done during type checking?

nikomatsakis (Dec 13 2018 at 19:39, on Zulip):

let's make a new topic I guess

Alexander Regueiro (Dec 13 2018 at 19:39, on Zulip):

yeah

Alexander Regueiro (Dec 13 2018 at 19:39, on Zulip):

I just thought that. :-P

nikomatsakis (Dec 13 2018 at 19:39, on Zulip):

I'll edit these messages actually

Alexander Regueiro (Dec 13 2018 at 19:39, on Zulip):

#impl-trait-in-bindings already exists

nikomatsakis (Dec 13 2018 at 19:39, on Zulip):

yep done

Alexander Regueiro (Dec 13 2018 at 19:39, on Zulip):

yep :-)

Alexander Regueiro (Dec 13 2018 at 19:39, on Zulip):

ta

nikomatsakis (Dec 13 2018 at 19:40, on Zulip):

so anyway I imagine roughly that in the code which checks let

Alexander Regueiro (Dec 13 2018 at 19:40, on Zulip):

It would probably help to collect all your notes (and my musings) in a single place

nikomatsakis (Dec 13 2018 at 19:40, on Zulip):

we would want to inspect if (a) there is a type annotation and (b) that type references opaque types

Alexander Regueiro (Dec 13 2018 at 19:40, on Zulip):

maybe a collaborative MD doc like you were starting

nikomatsakis (Dec 13 2018 at 19:40, on Zulip):

not a bad idea

Alexander Regueiro (Dec 13 2018 at 19:40, on Zulip):

right now they're in a few GH issues, PRs, and Zulip I feel

nikomatsakis (Dec 13 2018 at 19:40, on Zulip):

yeah

nikomatsakis (Dec 13 2018 at 19:40, on Zulip):

always hard to figure out how to organize such things

Alexander Regueiro (Dec 13 2018 at 19:40, on Zulip):

indeed

Alexander Regueiro (Dec 13 2018 at 19:41, on Zulip):

can I let you do that later? since you probably understand your notes rather better than I do!

Alexander Regueiro (Dec 13 2018 at 19:41, on Zulip):

and the topic in general

Alexander Regueiro (Dec 13 2018 at 19:42, on Zulip):

but, about code that checks let...
"we would want to inspect if (a) there is a type annotation and (b) that type references opaque types" -- right

nikomatsakis (Dec 13 2018 at 19:42, on Zulip):

yeah

nikomatsakis (Dec 13 2018 at 19:42, on Zulip):

yes, and basically if those things are true, we'd instantiate said opaque types...

Alexander Regueiro (Dec 13 2018 at 19:42, on Zulip):

okay

nikomatsakis (Dec 13 2018 at 19:42, on Zulip):

winding up with another variant of the type that is used with the RHS

Alexander Regueiro (Dec 13 2018 at 19:42, on Zulip):

do we check for "type(rhs) <: T" at that point too though?

nikomatsakis (Dec 13 2018 at 19:42, on Zulip):

that's the high-level idea anyway

Alexander Regueiro (Dec 13 2018 at 19:43, on Zulip):

I guess that's already part of assignment type checking?

Alexander Regueiro (Dec 13 2018 at 19:43, on Zulip):

or maybe I misunderstand

nikomatsakis (Dec 13 2018 at 19:45, on Zulip):

@Alexander Regueiro I think we would be adding this into the check_decl_initializer function here

Alexander Regueiro (Dec 13 2018 at 19:45, on Zulip):

right, I'm familiar with that

nikomatsakis (Dec 13 2018 at 19:45, on Zulip):

or possible check_decl_local

Alexander Regueiro (Dec 13 2018 at 19:45, on Zulip):

but do we need an additional subtype check?

Alexander Regueiro (Dec 13 2018 at 19:45, on Zulip):

like above

nikomatsakis (Dec 13 2018 at 19:46, on Zulip):

no we would be using this same check that already exists,

nikomatsakis (Dec 13 2018 at 19:46, on Zulip):

except that the type we are using would not be the type the user wrote

nikomatsakis (Dec 13 2018 at 19:46, on Zulip):

but rather the 'instantiated' version

nikomatsakis (Dec 13 2018 at 19:46, on Zulip):

this logic may actually already be sort of correct I guess

nikomatsakis (Dec 13 2018 at 19:46, on Zulip):

I mean there is revealed_ty

nikomatsakis (Dec 13 2018 at 19:47, on Zulip):

so yeah the type-checker part maybe doesn't have to change at all

nikomatsakis (Dec 13 2018 at 19:48, on Zulip):

or maybe only minimally

nikomatsakis (Dec 13 2018 at 19:48, on Zulip):

the question is more how to ensure this "cast" winds up in the MIR

nikomatsakis (Dec 13 2018 at 19:48, on Zulip):

presumably it also wants to be added to the HAIR

Alexander Regueiro (Dec 13 2018 at 19:49, on Zulip):

makes sense

Alexander Regueiro (Dec 13 2018 at 19:49, on Zulip):

I kind of wrote that code before, with my initial attempt

nikomatsakis (Dec 13 2018 at 19:49, on Zulip):

right

Alexander Regueiro (Dec 13 2018 at 19:50, on Zulip):

I wonder if MIR needs to do its own type-checking to make sure the cast is valid?

nikomatsakis (Dec 13 2018 at 19:50, on Zulip):

so maybe what we want is to modify the HAIR lowering for let

Alexander Regueiro (Dec 13 2018 at 19:50, on Zulip):

hmm

Alexander Regueiro (Dec 13 2018 at 19:50, on Zulip):

good idea

nikomatsakis (Dec 13 2018 at 19:50, on Zulip):

so

nikomatsakis (Dec 13 2018 at 19:51, on Zulip):

you can see that we already put the user-given type into the pattern, here

nikomatsakis (Dec 13 2018 at 19:51, on Zulip):

we might want to do something similar, afterwards, basically wrapping the initializer with some kind of HideUserType sort of thing

Alexander Regueiro (Dec 13 2018 at 19:51, on Zulip):

yep

nikomatsakis (Dec 13 2018 at 19:51, on Zulip):

(presuming that impl trait is present)

nikomatsakis (Dec 13 2018 at 19:52, on Zulip):

this would then be converted into some suitable MIR cast

nikomatsakis (Dec 13 2018 at 19:52, on Zulip):

which the type-checker would know to instantiate the opaque types etc

Alexander Regueiro (Dec 13 2018 at 19:52, on Zulip):

yeah... probably I can reuse the MIR code I've written for that, or something similar.

Alexander Regueiro (Dec 13 2018 at 19:52, on Zulip):

the MIR type checker you mean?

nikomatsakis (Dec 13 2018 at 19:52, on Zulip):

we might want to do something similar, afterwards, basically wrapping the initializer with some kind of HideUserType sort of thing

this btw gets back to that point about the type annotation serving two roles: you see it appearing (in opaque form) as an ascription on the pattern, and then in the cast (the revealed form)

nikomatsakis (Dec 13 2018 at 19:52, on Zulip):

the MIR type checker you mean?

yes

nikomatsakis (Dec 13 2018 at 19:53, on Zulip):

yeah... probably I can reuse the MIR code I've written for that, or something similar.

yes

Alexander Regueiro (Dec 13 2018 at 19:53, on Zulip):

makes sense

nikomatsakis (Dec 13 2018 at 19:53, on Zulip):

the basic code structure you had was right, it was just not necessarily triggering at the right times

Alexander Regueiro (Dec 13 2018 at 19:53, on Zulip):

okay. appreciate that. that's enough for now I think. :-)

nikomatsakis (Dec 13 2018 at 19:53, on Zulip):

awesome

Alexander Regueiro (Dec 13 2018 at 19:53, on Zulip):

at least, until you get around to collating your notes

nikomatsakis (Dec 13 2018 at 19:53, on Zulip):

yep, I'll try to find some suitable github place for them -- or, maybe better yet, we can put some of this logic into a rustc-guide PR

nikomatsakis (Dec 13 2018 at 19:54, on Zulip):

hmm I wonder if having rustc-guide PRs that stay open and get updated as we shift the design actually makes sense

nikomatsakis (Dec 13 2018 at 19:54, on Zulip):

kind of "design guide in progress"

nikomatsakis (Dec 13 2018 at 19:54, on Zulip):

and/or land and get edited

nikomatsakis (Dec 13 2018 at 19:54, on Zulip):

ok, gotta run anyway, meeting time

Alexander Regueiro (Dec 13 2018 at 19:57, on Zulip):

yeah that could work :-)

Alexander Regueiro (Dec 13 2018 at 19:57, on Zulip):

and sure. I've left you some PMs, btw.

Last update: Nov 12 2019 at 16:30UTC