Stream: t-compiler

Topic: #53797 uniform path ambiguity


pnkfelix (Oct 18 2018 at 14:15, on Zulip):

oy

nikomatsakis (Oct 18 2018 at 14:15, on Zulip):

seems kind of problematic, esp. as I see uniform paths gaining in "popularity" in the recent lang discussion

pnkfelix (Oct 18 2018 at 14:16, on Zulip):

yeah

pnkfelix (Oct 18 2018 at 14:16, on Zulip):

maybe we should add T-lang to this ticket

nikomatsakis (Oct 18 2018 at 14:16, on Zulip):

it's also a 'false ambiguity'

nikomatsakis (Oct 18 2018 at 14:16, on Zulip):

since both paths lead to the same place

pnkfelix (Oct 18 2018 at 14:16, on Zulip):

and that ... should always be the case when this arises ... ?

pnkfelix (Oct 18 2018 at 14:17, on Zulip):

also, isn't it goofy that it highlights the extern crate criterion with "can refer to self::criterion" ?

nikomatsakis (Oct 18 2018 at 14:17, on Zulip):

I thnk it is saying

pnkfelix (Oct 18 2018 at 14:17, on Zulip):

maybe that's meant to say that the extern crate is creating that binding?

nikomatsakis (Oct 18 2018 at 14:18, on Zulip):

"the path might be referring to this, self::criterion"

nikomatsakis (Oct 18 2018 at 14:18, on Zulip):

yes, it's confusing

pnkfelix (Oct 18 2018 at 14:18, on Zulip):

okay

pnkfelix (Oct 18 2018 at 14:18, on Zulip):

ugh. Is this an RC2 blocker?

pnkfelix (Oct 18 2018 at 14:18, on Zulip):

I think it might be

nikomatsakis (Oct 18 2018 at 14:18, on Zulip):

I'm feeling like it is

pnkfelix (Oct 18 2018 at 14:18, on Zulip):

I'll make a note saying so

nikomatsakis (Oct 18 2018 at 14:18, on Zulip):

this is a very common scenario, no?

nikomatsakis (Oct 18 2018 at 14:18, on Zulip):

it's basically "all code that uses an extern crate at root"

pnkfelix (Oct 18 2018 at 14:19, on Zulip):

but if we don't use uniform_paths

pnkfelix (Oct 18 2018 at 14:19, on Zulip):

then it doesn't arise, right?

nikomatsakis (Oct 18 2018 at 14:19, on Zulip):

point is, we are future proofed

nikomatsakis (Oct 18 2018 at 14:19, on Zulip):

that is, we emit the errors

pnkfelix (Oct 18 2018 at 14:20, on Zulip):

I.e. you have to opt into that feature in order to see this in the first place... right?

nikomatsakis (Oct 18 2018 at 14:20, on Zulip):

no, you don't

pnkfelix (Oct 18 2018 at 14:20, on Zulip):

oh I see, I misunderstood

nikomatsakis (Oct 18 2018 at 14:20, on Zulip):

we emit errors if you do something that would be incompatible with opting in to that future (in Rust 2018)

pnkfelix (Oct 18 2018 at 14:20, on Zulip):

or

pnkfelix (Oct 18 2018 at 14:21, on Zulip):

hmm

eddyb (Oct 18 2018 at 14:21, on Zulip):

yeah this is worse since we turned on the future-proofing errors by default

nikomatsakis (Oct 18 2018 at 14:21, on Zulip):

seems like a big "oops"

nikomatsakis (Oct 18 2018 at 14:21, on Zulip):

otoh I don't know if we're going to fix it for RC2..?

nikomatsakis (Oct 18 2018 at 14:21, on Zulip):

I guess it depends how much of a plan you think you have :)

eddyb (Oct 18 2018 at 14:21, on Zulip):

well, the thing that should happen here is the paths should be rewritten to use ::crate_name::...;

pnkfelix (Oct 18 2018 at 14:21, on Zulip):

Do we need to alert the lang team to it?

eddyb (Oct 18 2018 at 14:21, on Zulip):

but I really want to know what @Vadim Petrochenkov thinks

eddyb (Oct 18 2018 at 14:22, on Zulip):

because he wants to eventually get rid of the canaries

eddyb (Oct 18 2018 at 14:22, on Zulip):

and I want to take that into account (if I even get to it before him)

nikomatsakis (Oct 18 2018 at 14:22, on Zulip):

well, the thing that should happen here is the paths should be rewritten to use ::crate_name::...;

I mean I guess we could just do that but it will also run afoul of https://github.com/rust-lang-nursery/rustfmt/issues/3104

eddyb (Oct 18 2018 at 14:22, on Zulip):

yeah that's a bug that should be fixed

eddyb (Oct 18 2018 at 14:23, on Zulip):

oh wow it's rustfmt reading the edition from the wrong place

eddyb (Oct 18 2018 at 14:23, on Zulip):

yeah that's... bad. but also, hang on

eddyb (Oct 18 2018 at 14:23, on Zulip):

why doesn't cargo fmt pass --edition?

eddyb (Oct 18 2018 at 14:23, on Zulip):

cargo doc passes --edition to rustdoc, right?

pnkfelix (Oct 18 2018 at 14:23, on Zulip):

@eddyb Am I right in understanding your statement as saying that the current bug description does not accurately convey how severe the problem actually is right now, due to the other future-proofing changes?

nikomatsakis (Oct 18 2018 at 14:23, on Zulip):

why doesn't cargo fmt pass --edition?

it might, I don't use that

eddyb (Oct 18 2018 at 14:23, on Zulip):

oh

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

I just have "fmt on save" enabled...

eddyb (Oct 18 2018 at 14:24, on Zulip):

that should be using the Cargo integration I think

pnkfelix (Oct 18 2018 at 14:24, on Zulip):

yeah this is worse since we turned on the future-proofing errors by default

By "your statement" i meant the above.

eddyb (Oct 18 2018 at 14:24, on Zulip):

if you mean the RLS VSCode plugin

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

...in emacs :P

eddyb (Oct 18 2018 at 14:24, on Zulip):

oh, welp

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

regardless, I think we should not be assuming outdated code by default, but I'l comment on the rustfmt issue :)

eddyb (Oct 18 2018 at 14:24, on Zulip):

@pnkfelix I mean it's possible to trigger the errors on stable which means we need to have a migration path

pnkfelix (Oct 18 2018 at 14:25, on Zulip):

okay

eddyb (Oct 18 2018 at 14:25, on Zulip):

I was going to do that before getting bogged down in a million things, and I failed to sync up with @Vadim Petrochenkov, which makes it harder to figure out the proper approach

pnkfelix (Oct 18 2018 at 14:25, on Zulip):

My current inclination is to not tag this with T-lang

nikomatsakis (Oct 18 2018 at 14:26, on Zulip):

I don't think it needs T-lang per se

eddyb (Oct 18 2018 at 14:26, on Zulip):

how far away is RC2?

nikomatsakis (Oct 18 2018 at 14:26, on Zulip):

not very :) Oct 25 is the official date

nagisa (Oct 18 2018 at 14:26, on Zulip):

what tagging this issue with T-lang would help with?

eddyb (Oct 18 2018 at 14:26, on Zulip):

and is this the only thing I need to prioritize? (ugh, type aliases)

eddyb (Oct 18 2018 at 14:26, on Zulip):

oh wow, RC2 is on my birthday. how quaint :P

nikomatsakis (Oct 18 2018 at 14:26, on Zulip):

it's the only RC2 blocker issue at present (type aliases are not on that list)

pnkfelix (Oct 18 2018 at 14:26, on Zulip):

@nagisa I was thinking it would potentially influence the current debate about which path semantics to adopt

nagisa (Oct 18 2018 at 14:26, on Zulip):

oh those aren’t decided yet hmm…

nikomatsakis (Oct 18 2018 at 14:27, on Zulip):

personally @eddyb I think I'd prefer if use foo were not considered ambiguous in this scenario

nikomatsakis (Oct 18 2018 at 14:27, on Zulip):

is that something we could plausibly do?

eddyb (Oct 18 2018 at 14:27, on Zulip):

okay so the plan is for me to find a way to emit changes to imports somewhere such that migration goes smoothly

pnkfelix (Oct 18 2018 at 14:27, on Zulip):

okay well ... I sort of want to keep moving in the meeting

Vadim Petrochenkov (Oct 18 2018 at 14:27, on Zulip):

What makes #53797 so important?

nikomatsakis (Oct 18 2018 at 14:27, on Zulip):

i.e., we understand that use self::foo and foo from the prelude go to the same place :)

eddyb (Oct 18 2018 at 14:28, on Zulip):

@nikomatsakis what criteria are you thinking of?

Vadim Petrochenkov (Oct 18 2018 at 14:28, on Zulip):

Can't it be delayed?

eddyb (Oct 18 2018 at 14:28, on Zulip):

uhhh

nikomatsakis (Oct 18 2018 at 14:28, on Zulip):

okay well ... I sort of want to keep moving in the meeting

good idea... since @Vadim Petrochenkov and @eddyb are here maybe we can break out a separate topic

eddyb (Oct 18 2018 at 14:28, on Zulip):

@nikomatsakis oh wait I fixed that...

eddyb (Oct 18 2018 at 14:28, on Zulip):

hang on, is it because extern crate has an extra level of indirection?

nikomatsakis (Oct 18 2018 at 14:28, on Zulip):

cc @Vadim Petrochenkov @eddyb

eddyb (Oct 18 2018 at 14:28, on Zulip):

so I assumed the error was legit, but this is a good point

nikomatsakis (Oct 18 2018 at 14:28, on Zulip):

the reason @Vadim Petrochenkov that it seems important for RC2 is that it basically breaks all migration, no?

pnkfelix (Oct 18 2018 at 14:28, on Zulip):

let me do an alpha rename of the point where this discussino started

eddyb (Oct 18 2018 at 14:29, on Zulip):

extern crate foo; use foo::x; should not show up as ambiguous

nikomatsakis (Oct 18 2018 at 14:29, on Zulip):

that is, any crate which uses extern crate foo (all of them) and has a use foo:: at the root level (many of them...)

eddyb (Oct 18 2018 at 14:29, on Zulip):

since those are the same entity

nikomatsakis (Oct 18 2018 at 14:29, on Zulip):

I agree

nikomatsakis (Oct 18 2018 at 14:29, on Zulip):

is it possible this is..just fixed?

eddyb (Oct 18 2018 at 14:29, on Zulip):

has anyone tried?

nikomatsakis (Oct 18 2018 at 14:29, on Zulip):

I haven't :P

eddyb (Oct 18 2018 at 14:30, on Zulip):

we still need to do migration for things that would error from future-proofing

eddyb (Oct 18 2018 at 14:30, on Zulip):

but I guess the priority is lower other than this pattern that shouldn't even lower

Vadim Petrochenkov (Oct 18 2018 at 14:30, on Zulip):

(Ok, I'll never be able to follow this chat real-time, so I'll look at the ticket later and write what I think there.)

nikomatsakis (Oct 18 2018 at 14:30, on Zulip):

that does seem lower priority

nikomatsakis (Oct 18 2018 at 14:30, on Zulip):

(the more general case)

nikomatsakis (Oct 18 2018 at 14:31, on Zulip):

can we do something hacky, like... just check the extern prelude...?

eddyb (Oct 18 2018 at 14:31, on Zulip):

(I wish I could easily split Zulip into two windows)

eddyb (Oct 18 2018 at 14:31, on Zulip):

@nikomatsakis I already check for distinct targets

eddyb (Oct 18 2018 at 14:31, on Zulip):

this was needed to avoid use foo; being... ambiguous with itself

eddyb (Oct 18 2018 at 14:32, on Zulip):

without special-casing it, e.g. use foo::{self as foo}; also works

nikomatsakis (Oct 18 2018 at 14:32, on Zulip):

oh yes I remember that :)

nikomatsakis (Oct 18 2018 at 14:32, on Zulip):

I am testing now @eddyb

eddyb (Oct 18 2018 at 14:33, on Zulip):

extern crate rayon; use rayon::Scope; works on nightly and beta

nikomatsakis (Oct 18 2018 at 14:33, on Zulip):

in 2018?

eddyb (Oct 18 2018 at 14:34, on Zulip):

yeah. lemme try switching on uniform paths

nikomatsakis (Oct 18 2018 at 14:34, on Zulip):

:cold_sweat: shew

eddyb (Oct 18 2018 at 14:34, on Zulip):

also works

nikomatsakis (Oct 18 2018 at 14:34, on Zulip):

I confirm it works in my test too

nikomatsakis (Oct 18 2018 at 14:34, on Zulip):

I'll close

nikomatsakis (Oct 18 2018 at 14:35, on Zulip):

well I guess @eddyb did you want to have an issue for conflicts in the more general case?

eddyb (Oct 18 2018 at 14:36, on Zulip):

I don't know shrug

eddyb (Oct 18 2018 at 14:36, on Zulip):

I'm glad this isn't a RC2 blocker

eddyb (Oct 18 2018 at 14:36, on Zulip):

I mean, frankly, nobody could've migrated :P

eddyb (Oct 18 2018 at 14:36, on Zulip):

if this was still around

eddyb (Oct 18 2018 at 14:36, on Zulip):

so it makes sense that it's fixed

nikomatsakis (Oct 18 2018 at 14:37, on Zulip):

confirm, this is why I was confused

nikomatsakis (Oct 18 2018 at 14:37, on Zulip):

I was afraid people had just stopped testing :)

nikomatsakis (Oct 18 2018 at 14:37, on Zulip):

I'm going to close the issue for now

nikomatsakis (Oct 18 2018 at 14:37, on Zulip):

if we want one for the more general case, we should make an example

eddyb (Oct 18 2018 at 14:38, on Zulip):

I don't think you can hit the more general case, actually

eddyb (Oct 18 2018 at 14:38, on Zulip):

because everything crate-local gets rewritten to use crate::...

nikomatsakis (Oct 18 2018 at 14:38, on Zulip):

that would be nice :) this is why I was saying we should get a concrete example

eddyb (Oct 18 2018 at 14:39, on Zulip):

I think what I actually wanted was a migration to uniform paths that removes self:: and whatnot

nikomatsakis (Oct 18 2018 at 14:39, on Zulip):

we could probably do that within the 2018 edition though

nikomatsakis (Oct 18 2018 at 14:39, on Zulip):

e.g., as an idiom lint

eddyb (Oct 18 2018 at 14:40, on Zulip):

yeah I'm not even sure what I was worried about now

nikomatsakis (Oct 18 2018 at 14:40, on Zulip):

though I'd be a bit reluctant until we know the full conventions we want (well, it's probably good to remove use self::)

eddyb (Oct 18 2018 at 14:42, on Zulip):

hang on, no, I remember now

eddyb (Oct 18 2018 at 14:48, on Zulip):

bad news https://github.com/rust-lang/rust/issues/53797#issuecomment-431038351

nikomatsakis (Oct 18 2018 at 14:50, on Zulip):

well

nikomatsakis (Oct 18 2018 at 14:50, on Zulip):

ok, let's re-open, but I think lower priority

nikomatsakis (Oct 18 2018 at 14:50, on Zulip):

at some point, to address a similar problem

nikomatsakis (Oct 18 2018 at 14:50, on Zulip):

I had planned to just collect a global list of all modules

nikomatsakis (Oct 18 2018 at 14:50, on Zulip):

if an extern crate were shadowed anywhere

nikomatsakis (Oct 18 2018 at 14:50, on Zulip):

I was going to conservatively rewrite to the absolute form

nikomatsakis (Oct 18 2018 at 14:50, on Zulip):

obviously imprecise but not wrong

nikomatsakis (Oct 18 2018 at 14:51, on Zulip):

and the presumption is that this doesn't arise that much in practice

nikomatsakis (Oct 18 2018 at 14:51, on Zulip):

that'd be a quick fix ;)

eddyb (Oct 18 2018 at 14:51, on Zulip):

aaah that's an interesting approach

eddyb (Oct 18 2018 at 14:51, on Zulip):

but "a list of all modules" is not exactly enough

nikomatsakis (Oct 18 2018 at 14:51, on Zulip):

yeah I figured that as I wrote it

nikomatsakis (Oct 18 2018 at 14:51, on Zulip):

well, list of all "possibly shadowing things"

eddyb (Oct 18 2018 at 14:52, on Zulip):

use crate_name::...; will touch anything in the type namespace

eddyb (Oct 18 2018 at 14:52, on Zulip):

while use crate_name; will, well, - wait, so, I was going to say "all namespaces" and that's accurate

eddyb (Oct 18 2018 at 14:53, on Zulip):

but your suggestion is more about crate-wide confusion between an internal module and a dependency

eddyb (Oct 18 2018 at 14:53, on Zulip):

for which you really only care about modules. it's not a migration strategy, it's a potential idiom

nikomatsakis (Oct 18 2018 at 14:53, on Zulip):

for which you really only care about modules. it's not a migration strategy, it's a potential idiom

I don't quite understand what you mean by that

eddyb (Oct 18 2018 at 14:54, on Zulip):

a migration strategy would mostly be "does this name resolve in the scope of the use? if so, use the absolute path form"

eddyb (Oct 18 2018 at 14:55, on Zulip):

I mean that "is there any module with this name anywhere in the crate" is more of a heuristic for "should I disambiguate the dependency crate everywhere" as a "lowering confusion at the level of an entire crate" idiom of sorts

eddyb (Oct 18 2018 at 14:55, on Zulip):

it's neither sufficient nor necessary for migration, but it's a neat idiom

Last update: Nov 16 2019 at 02:40UTC