Stream: t-compiler/help

Topic: `!` fallback lint


nikomatsakis (Nov 06 2019 at 23:01, on Zulip):

Hey @llogiq (cc @centril) -- so about the lint

nikomatsakis (Nov 06 2019 at 23:02, on Zulip):

actually i'm not sure if this is the best stream but .. why not

nikomatsakis (Nov 06 2019 at 23:02, on Zulip):

I don't have a ton of time just now but if we create an issue I could also leave notes there, is there one?

centril (Nov 06 2019 at 23:02, on Zulip):

Heh; just woke up -- Haven't had a chance to review the developments in the thread

centril (Nov 06 2019 at 23:02, on Zulip):

@nikomatsakis I made a tracking issue when changing the PR: https://github.com/rust-lang/rust/issues/65992

nikomatsakis (Nov 06 2019 at 23:02, on Zulip):

Ah, great

nikomatsakis (Nov 06 2019 at 23:02, on Zulip):

thanks

centril (Nov 06 2019 at 23:03, on Zulip):

(it is also used in the code in the PR to activate the fallback)

llogiq (Nov 06 2019 at 23:08, on Zulip):

Ok, so I understand you want the lint to reach into inference to see if ! gets inferred because of fallback and then see if there would be a different outcome without the fallback.

nikomatsakis (Nov 06 2019 at 23:10, on Zulip):

I created https://github.com/rust-lang/rust/issues/66173 to track the lint specifically

nikomatsakis (Nov 06 2019 at 23:12, on Zulip):

Ok, so I understand you want the lint to reach into inference to see if ! gets inferred because of fallback and then see if there would be a different outcome without the fallback.

well

nikomatsakis (Nov 06 2019 at 23:12, on Zulip):

so what I had in mind was

nikomatsakis (Nov 06 2019 at 23:12, on Zulip):

the simplest version would be

nikomatsakis (Nov 06 2019 at 23:12, on Zulip):

at the point where we decide to apply fallback to some type variable ?T

nikomatsakis (Nov 06 2019 at 23:12, on Zulip):

we can inspect the pending obligations and things to get a list of all the type variables that are related to ?T

nikomatsakis (Nov 06 2019 at 23:13, on Zulip):

that is effectively the set that are now falling back to () and which will fallback to !

nikomatsakis (Nov 06 2019 at 23:13, on Zulip):

we can then see where those type variables appear -- e.g., do they appear in the types of variables and things?

nikomatsakis (Nov 06 2019 at 23:13, on Zulip):

we could also see if there are pending obligations like ?T: Debug or something that are true for () but not for !, or where it might change behavior

nikomatsakis (Nov 06 2019 at 23:13, on Zulip):

I'm not sure exactly when we want to lint, that is an interesting question

nikomatsakis (Nov 06 2019 at 23:14, on Zulip):

but I guess the first step would be to get this "tainted" set

nikomatsakis (Nov 06 2019 at 23:14, on Zulip):

honestly I sort of suspect that anytime this fallback affects live code, something...fishy may be going on worth linting about, even if we don't change the fallback...

centril (Nov 06 2019 at 23:16, on Zulip):

I wonder how big the fallout would be if we removed the fallback altogether in a crater run

nikomatsakis (Nov 06 2019 at 23:18, on Zulip):

worth an experiment

nikomatsakis (Nov 06 2019 at 23:18, on Zulip):

except that I feel like I tried it once and it was a problem..?

nikomatsakis (Nov 06 2019 at 23:18, on Zulip):

can't remember

nikomatsakis (Nov 06 2019 at 23:18, on Zulip):

I know we tried removing i32 fallback and it was glorious...except for testing code :) we had to put it back eventually

centril (Nov 06 2019 at 23:18, on Zulip):

too long ago :slight_smile: crater queue is reasonably sized for it to be worth it I recon

nikomatsakis (Nov 06 2019 at 23:19, on Zulip):

yeah this was all like back in 2012 or something :)

llogiq (Nov 06 2019 at 23:24, on Zulip):

Ok will be looking into obligations.

nikomatsakis (Dec 02 2019 at 20:48, on Zulip):

Hey @llogiq -- did you ever get time to investigate? Also, I was curious if you'd want to look into experimenting with a fix for https://github.com/rust-lang/rust/issues/66757 ?

llogiq (Dec 06 2019 at 20:39, on Zulip):

As I wrote on github, I was pretty swamped with work. So, can you or @centril help me out a bit so I can hit the ground running?

centril (Dec 06 2019 at 20:39, on Zulip):

I'm not familiar enough with inference to provide help

llogiq (Dec 06 2019 at 20:50, on Zulip):

In that case, I'd be happy if @nikomatsakis could help me find that a type fell back to () and how to get its outstanding trait obligations to check if our bad case applies?

llogiq (Dec 10 2019 at 19:21, on Zulip):

@nikomatsakis do you have a minute to give a broad overview where to find the code to adapt?

nikomatsakis (Dec 11 2019 at 21:30, on Zulip):

@llogiq will try to do so soon...

llogiq (Jan 05 2020 at 17:53, on Zulip):

Happy new year, @nikomatsakis! Can you set aside a few minutes to give me a few code spans to look into for the type fallback and how to check the obligations?

Matthew Jasper (Jan 05 2020 at 20:03, on Zulip):

You can probably start by looking at fallback_if_possible (librustc_typeck/check/mod.rs:3305), next_diverging_ty_var (librustc/infer/mod.rs), and the usages of self.diverges and tcx.types.never in librustc_typeck/check/*.

nikomatsakis (Jan 08 2020 at 21:08, on Zulip):

@llogiq yes! maybe we can schedule a time for this to do a little pair programming and get started?

nikomatsakis (Jan 15 2020 at 21:43, on Zulip):

@llogiq ok! Here is a zoom url we can use

nikomatsakis (Jan 15 2020 at 21:43, on Zulip):

(others are welcome to join if they want...)

nikomatsakis (Jan 15 2020 at 23:03, on Zulip):

LOL, well, we definitely get warnings...

nikomatsakis (Jan 15 2020 at 23:03, on Zulip):
psyche. rustc +rust-1-stage2 ~/tmp/issue_66173.rs
warning: resulted from diverging fallback: !
  --> /home/nmatsakis/tmp/issue_66173.rs:15:13
   |
15 |       let _ = if true {
   |  _____________^
16 | |         unconstrained_return()
17 | |     } else {
18 | |         panic!()
19 | |     };
   | |_____^

warning: resulted from diverging fallback: !
  --> /home/nmatsakis/tmp/issue_66173.rs:15:21
   |
15 |       let _ = if true {
   |  _____________________^
16 | |         unconstrained_return()
17 | |     } else {
   | |_____^

warning: resulted from diverging fallback: !
  --> /home/nmatsakis/tmp/issue_66173.rs:16:9
   |
16 |         unconstrained_return()
   |         ^^^^^^^^^^^^^^^^^^^^^^

warning: resulted from diverging fallback: fn() -> ! {unconstrained_return::<!>}
  --> /home/nmatsakis/tmp/issue_66173.rs:16:9
   |
16 |         unconstrained_return()
   |         ^^^^^^^^^^^^^^^^^^^^
... // continues for some time
Last update: Sep 18 2020 at 21:00UTC