Skip to content

Allow foo.rs to be parent to foo/bar.rs #39702

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

Closed

Conversation

withoutboats
Copy link
Contributor

Prior to this commit, in order for a module to have submodules, it had
to be at the /foo/mod.rs filepath. After this commit, modules at the
/foo.rs filepath have submodules located in the /foo/ directory.

@rust-highfive
Copy link
Contributor

r? @pnkfelix

(rust_highfive has picked a reviewer for you, use r? to override)

@withoutboats
Copy link
Contributor Author

withoutboats commented Feb 9, 2017

Not sure if this needs an RFC or not.

As written this doesn't work (and doesn't even get past stage 1), because #[path] attributes need to search paths from the directory the file is in, but this causes them to search in a subdirectory.

For example, in src/librustc_borrowck/borrowck/move_data.rs:

#[path="fragments.rs"]
mod fragments;

Needs to search for that file at src/librustc_borrowck/borrowck/fragments.rs, but instead on this branch src/librustc_borrowck/borrowck/mock_data/fragments.rs is searched.

I'll keep iterating on it though.

EDIT: Fixed this.

@withoutboats
Copy link
Contributor Author

Not sure if this needs an RFC or not.

Talked with the lang team, we agreed that this needs an RFC. I'll write one.

@withoutboats withoutboats force-pushed the more_flexible_mods branch 2 times, most recently from 88f82fa to 6b276c7 Compare February 10, 2017 07:47
@withoutboats
Copy link
Contributor Author

Question for the RFC - Consider this code:

// src/lib.rs
#[path="bar.rs"]
mod foo;
// src/bar.rs (the `foo` module)
mod baz;

Should the baz module be searched for at src/foo/baz.rs or src/bar/baz.rs?

My inclination is to search at src/bar/baz.rs because if you chose #[path="bar/mod.rs"] that is where it would search.

@codyps
Copy link
Contributor

codyps commented Feb 10, 2017

On the paths: unless there is an existing example otherwise, it probably makes sense to keep them as being relative to the directory containing the file which contains the #[path="..."] attribute. Otherwise thinking about path gets harder than it really needs to be.

@KalitaAlexey
Copy link
Contributor

I'd like to say against this feature because it adds complexity without any benefits.

@codyps
Copy link
Contributor

codyps commented Feb 10, 2017

I don't mind it so much: it could make moving from a single-file module (src/foo.rs) to a multi-file module (src/foo.rs + src/foo/blah.rs) not require a renaming of a file, which makes moving to multi-file modules easier (smaller diff & better vcs logs).

@withoutboats
Copy link
Contributor Author

On the paths: unless there is an existing example otherwise, it probably makes sense to keep them as being relative to the directory containing the file which contains the #[path="..."] attribute. Otherwise thinking about path gets harder than it really needs to be.

The way the path attribute is parsed can't change. The question is whether path'd modules have submodules at the subdirectory matching their module name or their path name.

@codyps
Copy link
Contributor

codyps commented Feb 10, 2017

Ah yes, I see what you were getting at. In that case, your suggestion sounds reasonable.

Prior to this commit, in order for a module to have submodules, it had
to be at the `/foo/mod.rs` filepath. After this commit, moudles at the
`/foo.rs` filepath have submodules located in the `/foo/` directory.
@withoutboats
Copy link
Contributor Author

After considering all the cases, what this implements (and the RFC will propose) is that a path attribute will contain submodules using these rules:

  • foo/mod.rs will have submodules in foo/
  • foo/bar.rs will have submodules in bar if "bar.rs" is valid unicode matching the regex .+\.rs
  • Any other file name (not ending in .rs, not valid unicode, just .rs with no prefix) cannot have submodules, just as non-mod.rs modules cannot have today.

Again this only applies to modules at path attribute locations; I would be surprised if any Rust code in the wild has a module falling under the third category.

@nagisa
Copy link
Member

nagisa commented Feb 11, 2017

Question for the RFC - Consider this code:

Take arbitrary stance in the RFC and pose this question in the RFC’s unresolved questions section.

@bors
Copy link
Collaborator

bors commented Feb 21, 2017

☔ The latest upstream changes (presumably #39765) made this pull request unmergeable. Please resolve the merge conflicts.

@pnkfelix
Copy link
Member

pnkfelix commented Mar 6, 2017

@withoutboats any update on the status of the RFC being drafted? Should I close this PR in the meantime?

@withoutboats
Copy link
Contributor Author

Yea I'll close this for now. Hope to get back to the RFC in the next few weeks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants