resolve: Fix variant namespacing

[petrochenkov]

Tuple and unit variants from other crates weren't put into type namespace.
Now variant namespacing is aligned with struct namespacing and is not affected by the variant's crate of origin (struct -> type, tuple/unit -> type/value).
Additionally, struct variants from other crates are put into value namespace (struct variants from local crate were already in it). This is not a necessity, but a future proofing measure.

This fix can result in some new shadowing errors in cross-crate scenarios, crater reports three regressions.
[breaking-change]

[rust-highfive]

r? @Aatch

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

[jonas-schievink]

cc me (this might fix a bug I've encountered)

[alexcrichton]

r? @nrc

[nrc]

I wonder if it is worth leaving struct variants in the value namespace? I don't suppose we'll ever need it, but it is future-proofing (e.g., if we someday get named arguments for functions, then we could make struct variants generate functions like other variants).

[nrc]

Should probably decide on the behaviour before we give it a crater run, but it will need one before landing

[petrochenkov]

Adding struct variants into the value namespace (including cross-crate) will give us the "maximum breakage mode", best suited for evaluation with crater. I'll add a second commit doing this.

[petrochenkov]

Regarding keeping struct variants in the value namespace in the long term, I'm mildly against it.
Correspondence between behavior of {}-structs and {}-variants feels important (especially if variants become types) and we can't put {}-structs into value namespace now.

The pattern

struct S { fields: Fields }
fn S(args: Args) -> S { ... } // Constructor

is not especially idiomatic, but it exists and disallowing it would (presumably) break too much code.
If functions with named arguments become a thing someday, it would be strange to generate them for variants, but not for structs (but maybe it could be considered a "best-effort").

[petrochenkov]

Updated with all variants in both namespaces.

[nrc]

@brson could you crater this please?

[nrc]

@petrochenkov we often have this issue - do we want to be consistent between structs and struct variants or struct variants and other variants? I don't have an opinion in this case, but if we can get away with leaving our options open, I would like that.

[bors]

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

[petrochenkov]

Rebased.

[alexcrichton]

Starting a crater run now

[alexcrichton]

Looks like there are three regressions (legitimate ones I believe)

[petrochenkov]

pnet_macros_plugin-0.1.0 - conflict of struct-struct syntax::ast::Delimited and tuple-variant syntax::ast::TokenTree::Delimited in type namespace (this crate doesn't seem to work on stable).

rotor-http-0.4.0 - conflict of struct-struct server::request::Head and unit-variant hyper::method::Method::Head in type namespace (this crate works on stable).

tiled-0.1.4 - conflict of struct-struct std::io::Error and tuple-variant xml::reader::events::XmlEvent::Error in type namespace (this crate seems to work on stable).

Zero regressions due to future-proofing of struct-variants suggested by @nrc

cc @mrmonday @tailhook @mattyhall

[mrmonday]

Hi, feel free to ping me about the pnet_macros_plugin failure - I don't know what you need from me specifically.

@petrochenkov, you are correct, pnet_macros_plugin will only work on nightly - it's an optional dependency of libpnet, which is only pulled in when the nightly feature is enabled. It is literally a wrapper around pnet_macros but with plugin = true added to the Cargo.toml. By default, libpnet uses syntex, rather than the plugin (I keep it around because build times are 3x faster when using plugins directly rather than syntex).

[nrc]

cc @rust-lang/lang

I'd be happy to take this and submit PRs to the breaking crates above. What do the rest of the team think?

[nrc]

note: I do think of this as qualifying as a bug fix

[aturon]

I'm on board with future-proofing here.

[tailhook]

Thanks to @brson, the bug in rotor-http is fixed now. This case looks more like an oversight rather than a valid use case. And I'm happy that compiler catches it now.

[bors]

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

[petrochenkov]

Rebased.

[nrc]

@bors: r+

(discussed at lang team meeting today)

[bors]

📌 Commit ff6b0aa has been approved by nrc

[bors]

⌛ Testing commit ff6b0aa with merge 54475e9...

[bors]

☀️ Test successful - auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-64-debug-opt, auto-linux-64-nopt-t, auto-linux-64-opt, auto-linux-64-x-android-t, auto-linux-cross-opt, auto-linux-musl-64-opt, auto-mac-32-opt, auto-mac-64-nopt-t, auto-mac-64-opt, auto-win-gnu-32-nopt-t, auto-win-gnu-32-opt, auto-win-gnu-64-nopt-t, auto-win-gnu-64-opt, auto-win-msvc-32-opt, auto-win-msvc-64-opt

[alexcrichton]

Just ran a crate report against nightly, and I wanted to confirm, but the regression of the tiled crate was expected, here, right? This didn't intend to add support to ensure it kept compiling?

[petrochenkov]

Yes, this is an expected regression.
A fix to this crate was merged in February, but the published version still isn't updated.

[alexcrichton]

Ok, thanks @petrochenkov!

[mattyhall]

I've pushed the new version.