Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stabilize uniform paths on Rust 2018 (technical details) #56417

Closed
petrochenkov opened this issue Dec 1, 2018 · 5 comments · Fixed by #56759
Closed

Stabilize uniform paths on Rust 2018 (technical details) #56417

petrochenkov opened this issue Dec 1, 2018 · 5 comments · Fixed by #56759
Assignees
Labels
A-resolve Area: Path resolution B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@petrochenkov
Copy link
Contributor

petrochenkov commented Dec 1, 2018

#55618 is the high-level and high-volume thread on whether we want uniform import paths in the language in general or not.
This is a more focused issue about how exactly stabilization will happen (assuming it will happen in general).

First, regarding timing of the stabilization.
I propose to test uniform paths for 4-5 weeks starting from Dec 07 (Rust 2018 release, 1.31 stable), and then backport their stabilization on 1.32 beta if everything is good.

Second, regarding sub-features and partial stabilization.
Imports use NAME or use NAME::... in the uniform path model can refer to various entities, not all of which may be expected or were discussed.
Here's the list:

  • Items defined in named or unnamed modules (mod m { struct NAME; }, fn f() { struct NAME; }).
    No known issues to resolve before stabilization.
  • Macros from other crates imported with #[macro_use] extern crate ..., for example use panic from the standard library.
    No known issues to resolve before stabilization.
  • Extern crate names from extern prelude, for example use std or use regex.
    No known issues to resolve before stabilization.
  • Names from the standard library prelude, for example use Vec.
    No known issues to resolve before stabilization.
  • Built-in types, for example use u8.
    No known issues to resolve before stabilization.
  • Built-in macros, for example use env.
    Currently an error due to some (fixable) implementation details of built-in macros.
    No known issues to resolve before stabilization (after the error is removed).
  • Macros defined with macro_rules! in the same crate, e.g. macro_rules! mac {()=>()} use mac as pac
    Unresolved question: what visibility to attach to these macros, in other words - how far can they be reexported with pub use?
    Proposal: treat #[macro_export] macro_rules! { ... } as pub, treat other macro_rules! { ... } as pub(crate).
    Motivation: 1) #[macro_export] are indeed visible from other crates, 2) non-#[macro_export] macros themselves are indeed potentially visible from the whole crate, it depends on the containing module whether to actually let them out or not (similarly to public items in private modules and their potential reexports).
  • Built-in attributes, for example use inline.
    Issue: even if inline is reimported under some other name, e.g. use inline as my_inline, my_inline won't be treated as inline by the compiler. Even later stages of the compiler work with attributes at token level, not using resolution results. That's not good in general, ideally attributes should be lowered into some semantic form somewhere around AST -> HIR conversion.
    This means a compatibility hazard, for example #[my_repr(D)] fn f() {} would be accepted and ignored if attributes are treated syntactically (assuming use repr as my_repr), but would be an error if attributes are treated semantically based on their resolution.
    On the other hand, if use builtin_attr is still feature-gated, then things like use proc_macro (or use ignore as recently reported) will be feature gated as well (use imports in all namespaces, and proc_macro is not only a crate, but also a built-in attribute).
    Proposal: Allow imports of built-in attributes, but prohibit actually using names imported this way in attribute positions.
  • Derive helper attributes registered by derive macros, for example use serde attribute registered by Serialize macro.
    Not fully implemented, so imports can never refer to them.
    Issue (once fully implemented): similarly to built-in attributes, derive helpers reimported under other name will be unrecognizable by their respective proc macros, because proc macros work at token level.
    Proposal: Allow imports of derive helper attributes, but prohibit actually using names imported this way in attribute positions.
  • "Tool modules" in tool attributes, like rusfmt in rustfmt::skip.
    Issue: similarly to built-in attributes, tool modules reimported under other name will be unrecognizable by their respective tools, because tools work at token level.
    Proposal: Allow imports of tool modules, but prohibit actually using names imported this way in attribute paths.
@petrochenkov
Copy link
Contributor Author

petrochenkov commented Dec 1, 2018

So, we need to either:

  • decide on visibility of macro_rules and future-proofing of attribute re-imports, and stabilize everything, or
  • do not decide and stabilize everything except for imports referring to macro_rules / built-in attributes / tool modules.

@petrochenkov petrochenkov added A-resolve Area: Path resolution T-lang Relevant to the language team, which will review and decide on the PR/issue. B-unstable Blocker: Implemented in the nightly compiler and unstable. labels Dec 1, 2018
@Centril Centril added the C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. label Dec 2, 2018
@Centril Centril self-assigned this Dec 2, 2018
@petrochenkov petrochenkov self-assigned this Dec 2, 2018
@petrochenkov
Copy link
Contributor Author

#56759 implements suggestions for macro_rules and attribute import future-proofing.

bors added a commit that referenced this issue Jan 12, 2019
Stabilize `uniform_paths`

Address all the things described in #56417.

Assign visibilities to `macro_rules` items - `pub` to `macro_export`-ed macros and `pub(crate)` to non-exported macros, these visibilities determine how far these macros can be reexported with `use`.

Prohibit use of reexported inert attributes and "tool modules", after renaming (e.g. `use inline as imported_inline`) their respective tools and even compiler passes won't be able to recognize and properly check them.

Also turn use of uniform paths in 2015 macros into an error, I'd prefer to neither remove nor stabilize them right away because I still want to make some experiments in this area (uniform path fallback was added to 2015 macros used on 2018 edition in #56053 (comment)).

UPDATE: The last commit also stabilizes the feature `uniform_paths`.

Closes #53130
Closes #55618
Closes #56326
Closes #56398
Closes #56417
Closes #56821
Closes #57252
Closes #57422
bors added a commit that referenced this issue Jan 12, 2019
Stabilize `uniform_paths`

Address all the things described in #56417.

Assign visibilities to `macro_rules` items - `pub` to `macro_export`-ed macros and `pub(crate)` to non-exported macros, these visibilities determine how far these macros can be reexported with `use`.

Prohibit use of reexported inert attributes and "tool modules", after renaming (e.g. `use inline as imported_inline`) their respective tools and even compiler passes won't be able to recognize and properly check them.

Also turn use of uniform paths in 2015 macros into an error, I'd prefer to neither remove nor stabilize them right away because I still want to make some experiments in this area (uniform path fallback was added to 2015 macros used on 2018 edition in #56053 (comment)).

UPDATE: The last commit also stabilizes the feature `uniform_paths`.

Closes #53130
Closes #55618
Closes #56326
Closes #56398
Closes #56417
Closes #56821
Closes #57252
Closes #57422
@shepmaster
Copy link
Member

* Built-in macros, for example `use env`.
  Currently an error due to some (fixable) implementation details of built-in macros.
  No known issues to resolve before stabilization (after the error is removed).

Is this progress being tracked somewhere else?

@petrochenkov
Copy link
Contributor Author

@shepmaster

Is this progress being tracked somewhere else?

I don't think so.

The issue is that built-in macros use CrateNum::BuiltinMacros in its DefId and it cannot leave resolve without causing a bunch of ICEs (it's a hack in general).
They should probably use LOCAL_CRATE instead and CrateNum::BuiltinMacros should be removed.

@petrochenkov
Copy link
Contributor Author

Made an issue for built-in macro imports - #61687.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-resolve Area: Path resolution B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants