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

MCP: Change path attribute behavior of modules. #89

Closed
crlf0710 opened this issue Apr 3, 2021 · 6 comments
Closed

MCP: Change path attribute behavior of modules. #89

crlf0710 opened this issue Apr 3, 2021 · 6 comments
Labels
disposition-close The FCP starter wants to close this final-comment-period The FCP has started, most (if not all) team members are in agreement major-change Major change proposal T-lang to-announce Not yet announced MCP proposals

Comments

@crlf0710
Copy link
Member

crlf0710 commented Apr 3, 2021

Proposal

Summary and problem statement

Change the file lookup rules of path attributes on modules to better align with current Rust module hierarchy organization.

Motivation, use-cases, and solution sketches

Currently path attribute behavior is explained in the two tables in reference. One thing that is not explained on the page is where will mod c 's child module be lookuped at.

#[path = "foo.rs"]
mod c;

////////////
// foo.rs
mod d;
Source File c's File Location c's Module Path c's child file location
src/a/b.rs src/a/foo.rs crate::a::b::c src/a/d.rs
src/a/mod.rs src/a/foo.rs crate::a::c src/a/d.rs
mod inline {
    #[path = "foo.rs"]
    mod c;
}

////////////
// foo.rs
mod d;
Source File c's File Location c's Module Path c's child file location
src/a/b.rs src/a/b/inline/foo.rs crate::a::b::inline::c src/a/b/inline/d.rs
src/a/mod.rs src/a/inline/foo.rs crate::a::inline::c src/a/inline/d.rs

I believe this is due to heritage of edition 2015, where mod c should actually be an mod.rs file if it has non-inline child modules. However this seems really strange under 2018 edition. I hope this could be fixed under 2021 edition.

One possible solution:

  • Find mod c's child modules at the current location only when c is a "mod-rs" source file. Otherwise find its child within "foo" subdirectory.

Another possible solution:

  • Always find its child within "foo" subdirectory
  • Add a "foo/" (ends with slash) grammar for path attribute, find mod.rs and its children following 2015 edition from there.

Prioritization

  • Not very urgent, but current behavior seems inconsistent and changing it is a breaking change, so it might be nice if such changes are done during an edition boundary.

Links and related work

Initial people involved

What happens now?

This issue is part of the experimental MCP process described in RFC 2936. Once this issue is filed, a Zulip topic will be opened for discussion, and the lang-team will review open MCPs in its weekly triage meetings. You should receive feedback within a week or two.

This issue is not meant to be used for technical discussion. There is a Zulip stream for that. Use this issue to leave procedural comments, such as volunteering to review, indicating that you second the proposal (or third, etc), or raising a concern that you would like to be addressed.

@crlf0710 crlf0710 added T-lang major-change Major change proposal labels Apr 3, 2021
@rustbot
Copy link
Collaborator

rustbot commented Apr 3, 2021

This issue is not meant to be used for technical discussion. There is a Zulip stream for that. Use this issue to leave procedural comments, such as volunteering to review, indicating that you second the proposal (or third, etc), or raising a concern that you would like to be addressed.

@rustbot rustbot added the to-announce Not yet announced MCP proposals label Apr 3, 2021
@joshtriplett
Copy link
Member

See also rust-lang/rust#35016

@nikomatsakis
Copy link
Contributor

We discussed this in our 2021-04-20 meeting. While there were many folks who felt that the current behavior could be surprising, the overall consensus was that in order to make changes here we would want to see:

  • More evidence of users actively getting confused by this and having a hard time resolving that confusion (the existing issues contain relatively few comments, for example, and they are relatively old).
  • Examples of things that require path -- maybe we can just invent other patterns to avoid path instead?

Marking as disposition close.

@nikomatsakis nikomatsakis added disposition-close The FCP starter wants to close this final-comment-period The FCP has started, most (if not all) team members are in agreement labels Apr 20, 2021
@crlf0710
Copy link
Member Author

For the second bullet point, i'd list a few:

  1. The just stablized non-ascii-idents feature requires path attribute to be able to use non-inline module with non-ascii names.
  2. code that tries to "paper over" platform differences requires path attribute to "source" one module from different files, like https://github.com/rust-windowing/winit/blob/1c4d6e7613c3a3870cecb4cfa0eecc97409d45ff/src/platform_impl/mod.rs

@nikomatsakis
Copy link
Contributor

nikomatsakis commented Apr 22, 2021

@crlf0710 we did discuss this latter point: we were wondering if it was even worth trying to make a feature just for this case. That said, the question is--- how often do those scenarios arise inside of an inline module? (And how problematic is the current behavior in that case? After all, if one knows what to expect, they can do ../ if desired...)

@nikomatsakis
Copy link
Contributor

The FCP has expired, so I've closed the issue. Thanks @crlf0710 for the suggestion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
disposition-close The FCP starter wants to close this final-comment-period The FCP has started, most (if not all) team members are in agreement major-change Major change proposal T-lang to-announce Not yet announced MCP proposals
Projects
None yet
Development

No branches or pull requests

4 participants