Possibly incorrect syntax Enum::<Type, Params>::Variant allowed since 1.33

[estebank]

A coworker recently identified the following case:

pub enum Foo<'a, T> {
    Struct {},
    Tuple(),
    Unit,
    Usage(&'a T),
}
pub fn main() {
    let foo = Foo::<String>::Unit;
    let foo = Foo::<String>::Tuple();
    let foo = Foo::<String>::Struct {};
}

This used to fail because of the lack of implicit bound T: 'a:

error[E0309]: the parameter type `T` may not live long enough
 --> <source>:5:11
  |
1 | pub enum Foo<'a, T> {
  |                  - help: consider adding an explicit lifetime bound `T: 'a`...
...
5 |     Usage(&'a T),
  |           ^^^^^
  |
note: ...so that the reference type `&'a T` does not outlive the data it points at
 --> <source>:5:11
  |
5 |     Usage(&'a T),
  |           ^^^^^

After fixing that, it complains about the type params:

error[E0109]: type parameters are not allowed on this type
 --> <source>:8:21
  |
8 |     let foo = Foo::<String>::Unit;
  |                     ^^^^^^ type parameter not allowed

error[E0109]: type parameters are not allowed on this type
 --> <source>:9:21
  |
9 |     let foo = Foo::<String>::Tuple();
  |                     ^^^^^^ type parameter not allowed

error[E0109]: type parameters are not allowed on this type
  --> <source>:10:21
   |
10 |     let foo = Foo::<String>::Struct {};
   |                     ^^^^^^ type parameter not allowed

Starting in 1.33, all but the Struct path are accepted:

error[E0110]: lifetime arguments are not allowed on this entity
  --> <source>:10:15
   |
10 |     let foo = Foo::<String>::Struct {};
   |               ^^^^^^^^^^^^^^^^^^^^^ lifetime argument not allowed

error[E0107]: wrong number of lifetime arguments: expected 1, found 0
  --> <source>:10:15
   |
10 |     let foo = Foo::<String>::Struct {};
   |               ^^^^^^^^^^^^^^^^^^^^^ expected 1 lifetime argument

The correct code for this would of course be:

pub enum Foo<'a, T> {
    Struct {},
    Tuple(),
    Unit,
    Usage(&'a T),
}
pub fn main() {
    let foo = Foo::Unit::<String>;
    let foo = Foo::Tuple::<String>();
    let foo = Foo::Struct::<String> {};
}

My questions:

• Was this change in behavior (accepting some of these paths) intended?
• If so, was it intended to deny only one of these cases?
• If this change wasn't intended, should we try to reintroduce it or grandfather it in?

Depending on the answer to those questions, the solution to this ticket could be:

• Make the compiler accept the let foo = Foo::Struct::<String> {}; case
• Deny all cases with a suggestion pointing at the right syntax

cc @Centril

[eddyb]

cc @petrochenkov @varkor

[eddyb]

The correct code for this would of course be:

Not anymore, it was never "correct", it was at best a historical accident.
IMO it should've been changed once Enum::Variant became a thing, a long time ago
(before then, enum X { Y } placed X and Y in the surrounding scope).

---

Looks like the E::<T>::V {...} form is broken, and only wrt lifetimes?
Is it possible HIR lowering injects '_ placeholders in the wrong positions?

[Centril]

centril@centrilnas2:~/programming/rust/rust-gamma$ rustc +nightly foo.rs -Z unpretty=hir
#[prelude_import]
use ::std::prelude::v1::*;
#[macro_use]
extern crate std;
pub enum Foo<'a, T> {
    Struct {
    },
    Tuple(),
    Unit,
    Usage(&'a T),
}
pub fn main() {
                  let foo = Foo::Unit::<String>;
                  let foo = Foo::Tuple::<String>();
                  let foo = Foo::Struct::<, String>{};
              }

[eddyb]

Note the Foo::Struct::<, String>, there's a '_ in there, but with an empty name, because it's artificial (HIR pretty-printing should probably show '_ there).

So HIR lowering is treating Expr::Path and Expr::Struct differently in an incorrect way.

[estebank]

Answering my own questions based on the conversations on discord. All of the following should be allowed:

pub enum Foo<'a, T> {
    Struct {},
    Tuple(),
    Unit,
    Usage(&'a T),
}
pub fn main() {
    let foo = Foo::<String>::Unit;
    let foo = Foo::<String>::Tuple();
    let foo = Foo::<String>::Struct {};
    let foo = Foo::Unit::<String>;
    let foo = Foo::Tuple::<String>();
    let foo = Foo::Struct::<String> {};
}

Based on FIXMEs and the conversation, the later 3 should be linted against but must remain valid for backwards compatibility.

#69363 fixes this ticket.

[petrochenkov]

ping in #69356 (comment)

[triagebot] The discussion is continued on the PR (#69363), which is currently assigned to me.