parser: simplify directory ownership semantics

[jseyfried]

This PR simplifies the semantics of "directory ownership". After this PR,

• an "out of line" module without a #[path] attribute (e.g. mod foo;) is allowed iff its parent module/block (whichever is nearer) is a directory owner,
• an out of line module is a directory owner iff its corresponding file is named mod.rs (c.f. comment),
• a block is never a directory owner (c.f. libsyntax: Restrict where non-inline modules can appear (fixes #29765) #31534), and
• an inline module is a directory owner iff either
  • its parent module/block is a directory owner (again, c.f. libsyntax: Restrict where non-inline modules can appear (fixes #29765) #31534), or
  • it has a #[path] attribute (c.f. Allow more non-inline modules in blocks #36789).

These semantics differ from today's in three orthogonal ways:

• #[path = "foo.rs"] mod foo; is no longer a directory owner. This is a [breaking-change].
• Allow more non-inline modules in blocks #36789 is generalized to apply to modules that are not directory owners in addition to blocks.
• A macro-expanded out of line module is only allowed where an ordinary out of line module would be allowed. Today, we incorrectly allow macro-expanded out of line modules in modules that are not directory owners (but not in blocks). This is a [breaking-change].

Fixes #32401.
r? @nikomatsakis

[jseyfried]

cc @rust-lang/lang @matklad

[nikomatsakis]

Started a crater run.

[nikomatsakis]

Results: https://gist.github.com/nikomatsakis/ad150e91da19d7a86d8501a11f2b0a0c

• 6760 crates tested: 4753 working / 1638 broken / 25 regressed / 2 fixed / 342 unknown.

Root regressions to be investigated:

• petgraph-0.4.1 (before) (after) -- looks like a genuine regression
• ring-0.5.3 (before) (after) -- looks like a genuine regression
• gtld-data-0.4.1 (before) (after) -- some kind of weird OpenSSL thing I don't understand
• imgur-0.5.0 (before) (after) -- some kind of weird OpenSSL thing I don't understand
• atomic-0.3.3 (before) (after) -- looks like a genuine regression
• presto-0.1.0 (before) (after) -- some kind of weird OpenSSL thing I don't understand
• github-0.1.2 (before) (after) -- some kind of weird OpenSSL thing I don't understand
• cargo-deadlinks-0.1.0 (before) (after) -- some kind of weird OpenSSL thing I don't understand

[nikomatsakis]

@jseyfried so at least a few of those are real regr. We have to decide now: to warning period or what.

cc @rust-lang/lang -- this PR makes some changes to rationalize various aspects of when a module "has a directory" and not (i.e., when you can do mod foo;). It breaks some libraries. What to do?

[brson]

@nikomatsakis the openssl errors are rust-lang/cargo#3268

[aturon]

@nikomatsakis This looks like a good case for applying the general guidelines. In general I'm 👍 going forward with these changes. How hard is a warning period to do in this case? If it's relatively easy, we should do it.

[brson]

I'd encourage you to make any breakage here a transitionary warning. We must continue to do our utmost to reduce breakage pain for users. Small breakage adds up.

[jseyfried]

It looks like all the breakage is due to #[path = "foo.rs"] mod foo; no longer being a directory owner (i.e. #32401 (comment)). A warning cycle for this is straightforward to implement.

[nikomatsakis]

Definitely we should do a warning period.

[jseyfried]

I started a warning cycle in the above commit. Since it's not easy to lint from the parser, it is just an ordinary warning with a note about future compatibility.

[nikomatsakis]

Hmm. I think we could do an actual lint here -- can't we leave a little note on the AST or something and then issue the lint later, in the lint pass?

At minimum, though, can you open a "forward compat" issue with the appropriate labels and structure and mention it in this message? Roughly follow the protocol described in RFC 1589

[jseyfried]

Sure, I'll make it a real lint.

[jseyfried]

Done.

[nikomatsakis]

ping @jseyfried -- still planning to make it a real lint?

[jseyfried]

@nikomatsakis just fininshed (got sidetracked by some higher-priority stuff)

[petrochenkov]

The lint is temporary, but still could you rename it to follow lint naming conventions?
Names are supposed to look good in allow/deny attributes, like deny(deprecated_directory_ownership), but not deny(directory_ownership).

[jseyfried]

@petrochenkov yeah, renamed to legacy_directory_ownership.

[bors]

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

[nikomatsakis]

@jseyfried r=me after rebase

[jseyfried]

@bors r=nikomatsakis

[bors]

📌 Commit fa8c53b has been approved by nikomatsakis

[bors]

⌛ Testing commit fa8c53b with merge 1c11ea3...

[bors]

☀️ Test successful - auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-64-cargotest, auto-linux-64-cross-freebsd, auto-linux-64-debug-opt, auto-linux-64-nopt-t, auto-linux-64-opt, auto-linux-64-opt-rustbuild, auto-linux-64-x-android-t, auto-linux-cross-opt, auto-linux-musl-64-opt, auto-mac-32-opt, auto-mac-64-opt, auto-mac-64-opt-rustbuild, auto-mac-cross-ios-opt, auto-win-gnu-32-opt, auto-win-gnu-32-opt-rustbuild, auto-win-gnu-64-opt, auto-win-msvc-32-opt, auto-win-msvc-64-cargotest, auto-win-msvc-64-opt, auto-win-msvc-64-opt-rustbuild
Approved by: nikomatsakis
Pushing 1c11ea3 to master...

[briansmith]

Sorry, I don't understand what I need to do to change in ring here. I don't want to name all the modules' files mod.rs because it's ridiculous to have dozens of files with the same name in one project. Of course some of them have submodules. What's the alternative?

[eddyb]

@briansmith You can declare the module hierarchy recursively, even the entire crate's modules in lib.rs, and the nesting matches the directory structure. Some (older?) rustc crates do this, if you need examples.
I believe you can also use #[path = "foo.rs"] mod foo; to manually enforce your own hierarchy.