Track stability of path components

[brson]

The current stability check only checks a few uses: functions and methods. There is much more needed for the coverage to be complete. In particular, every path contains multiple components which should all be checked. Simply checking paths would improve the accuracy of the analysis greatly.

This requires modifying resolve to record the def_id's of path components, not just the entire path.

cc @aturon

[aturon]

cc #8962

[aturon]

cc #8967 as well.

We will also need to think carefully about the user experience for the lint for mostly-stable libraries. We may want opt-out at the crate level, or a way of coalescing warnings.

[lifthrasiir]

Nominating for 1.0. This is an important feature that possibly affects several libraries over the course of stabilization after 1.0 (I've already hit this twice with rust-encoding, see above). Also note that this doesn't have a simple workaround with an exception of function.

[pnkfelix]

P-high, not 1.0.

(This is highly desired, but it is not going to block a release.)

[brson]

Renominating because the ability to correctly detect the stability level of user code has a high impact on the guarantees we can provide.

[pnkfelix]

Assigning 1.0 milestone (it is considered polish, rather than a deep backwards compat issue).

[aturon]

@alexcrichton did your patch address this? For some reason it seemed it would be harder, but I don't really know.

[alexcrichton]

Unfortunately no, my patch didn't address this. For any path we still only track the stability of the last component.

[pnkfelix]

We need to investigate how much of the stdlib is potentially affected by this to decide what to do next (and how to do it).

[alexcrichton]

Unfortunately not having this bug fixed means that paths like std::intrinsics::copy are now stable. The intrinsics module is unstable but the function is stable because it's reexported in std::ptr.

If we fix this we probably need to have at least a cycle of warnings before turning on the infrastructure.

[alexcrichton]

cc @jseyfried, @petrochenkov, do either of you know if recent work has made this easy to knock out?

Also removing @aturon as assignee as it's not actively being worked on

[petrochenkov]

I don't think this can be done now, too much breakage (?).

do either of you know if recent work has made this easy to knock out?

Nothing that I know about.
Stability checks can be moved to resolve in a similar way like path privacy checks were moved there. This will help to check reexport stability as well. This has a cost of turning resolve into hodgepodge of various random checks. Giving path segments node ids and recording def_id's of path components is probably too heavy of a hammer too (unless it's needed for something else).

[jseyfried]

Perhaps we could have resolve construct a map from paths to a list of the resolutions of the path's components (i.e. a list of Defs or DefIds) and use that map during the stability checking pass.

[alexcrichton]

I wouldn't necessarily be too worried about the breakage here as we could turn these new stability violations into warnings for quite some time. We basically just want to communicate to people that the API they are accidentally using is actually stable somewhere else, and it's in their own best interest to use that instead.

If it's very difficult to implement in the compiler, though, this could just be something the libs team keeps in mind when stabilizing APIs and we can avoid having unstable modules with stable contents as much as possible.

[SimonSapin]

Still relevant in rustc 1.25.0. This compiles even though the intrinsics module and the Float trait are both unstable.

extern crate core;
fn main() {
    unsafe { std::intrinsics::copy(&0, &mut 0, 1); }
    core::num::Float::is_nan(1.0);
}

[petrochenkov]

Now I think this was the correct solution:

Stability checks can be moved to resolve in a similar way like path privacy checks were moved there. This will help to check reexport stability as well.

[pnkfelix]

maybe we can file this under "maturity" (and try to aim a fix for the next edition...)