Spurious unreachable_patterns with macro

[lnicola]

Code

I tried this code:

pub enum TypeCtor {
    Bool,
    Slice,
    Array,
}

pub struct ApplicationTy {
    pub ctor: TypeCtor,
    pub parameters: (),
}

pub enum Ty {
    Apply(ApplicationTy),
}

macro_rules! ty_app {
    ($ctor:pat, $param:pat) => {
        crate::Ty::Apply(crate::ApplicationTy {
            ctor: $ctor,
            parameters: $param,
        })
    };
    ($ctor:pat) => {
        ty_app!($ctor, _)
    };
}

fn _foo(ty: Ty) {
    match ty {
        ty_app!(TypeCtor::Array, _st) | ty_app!(TypeCtor::Slice, _st) => {}
        _ => {}
    }

    // same as above, with the macro expanded
    match ty {
        Ty::Apply(crate::ApplicationTy {
            ctor: TypeCtor::Array,
            parameters: _st,
        })
        | Ty::Apply(crate::ApplicationTy {
            ctor: TypeCtor::Slice,
            parameters: _st,
        }) => {}
        _ => {}
    }
}

fn main() {}

I expected to see this happen: code should build fine

Instead, this happened: unreachable pattern in the first, but not on the second match

warning: unreachable pattern
  --> src/main.rs:18:9
   |
18 | /         crate::Ty::Apply(crate::ApplicationTy {
19 | |             ctor: $ctor,
20 | |             parameters: $param,
21 | |         })
   | |__________^
...
30 |           ty_app!(TypeCtor::Array, _st) | ty_app!(TypeCtor::Slice, _st) => {}
   |                                           ----------------------------- in this macro invocation
   |
   = note: `#[warn(unreachable_patterns)]` on by default
   = note: this warning originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)

warning: 1 warning emitted

Version it worked on

It most recently worked on:

rustc 1.48.0 (7eac88abb 2020-11-16)
binary: rustc
commit-hash: 7eac88abb2e57e752f3302f02be5f3ce3d7adfb4
commit-date: 2020-11-16
host: x86_64-unknown-linux-gnu
release: 1.48.0
LLVM version: 11.0

Version with regression

rustc 1.50.0-beta.1 (05b602367 2020-12-29)
binary: rustc
commit-hash: 05b6023675d77979637b04a350c85903fbf59257
commit-date: 2020-12-29
host: x86_64-unknown-linux-gnu
release: 1.50.0-beta.1

@rustbot modify labels: +regression-from-stable-to-beta -regression-untriaged

[SNCPlay42]

searched nightlies: from nightly-2020-12-09 to nightly-2020-12-29
regressed nightly: nightly-2020-12-20
searched commits: from f745834 to 1f5bc17
regressed commit: 1f5bc17

bisected with cargo-bisect-rustc v0.6.0

Host triple: x86_64-unknown-linux-gnu
Reproduce with:

cargo bisect-rustc --preserve --start=2020-12-09 --end=2020-12-29 -- check 

[lqd]

cc @Nadrieril

[apiraino]

Assigning P-medium as discussed as part of the Prioritization Working Group procedure and removing I-prioritize.

[jyn514]

Slightly minimized:

pub enum TypeCtor {
    Slice,
    Array,
}

pub struct ApplicationTy(TypeCtor);

macro_rules! ty_app {
    ($ctor:pat) => {
        ApplicationTy($ctor)
    };
}

fn _foo(ty: ApplicationTy) -> usize {
    return match ty {
        ty_app!(TypeCtor::Array) | ty_app!(TypeCtor::Slice) => 1,
    };

    // same as above, with the macro expanded
    match ty {
        crate::ApplicationTy(TypeCtor::Array)
        | crate::ApplicationTy(TypeCtor::Slice) => {}
    }
}

fn main() {}

If you remove ApplicationTy it stops giving the warning for some reason.

[Nadrieril]

Oh boy, this is annoying. I had found a clever scheme in #80104 to deal with unreachable sub-patterns, but it relies on the assumption that the different alternatives in an or-pattern will have different spans.
Here both alternatives get given the span of ApplicationTy in the macro def, which breaks my clever idea :(. I'll have to think of a new idea that doesn't rely on spans...

[jyn514]

@Nadrieril I think if you call source_callsite it will point to the pattern in the match instead.

[Nadrieril]

@jyn514 Sounds interesting! I'm not entirely sure it does what I want. What would source_callsite return if I called it on the span of 1 below? I would want to get the same span of 1, but I suspect it would return the span of the foo!() invocation.

macro_rules! foo {
    () => {
        (0 | 1)
    };
}

fn main() {
	match 0 {
		foo!() -> {}
	}
}

[jyn514]

Yup, that's right. My idea probably wouldn't work then, sorry.

[Nadrieril]

That gave me the idea for this cursed thing, where several different patterns really do have the same span. Unless I can distinguish a same span across different macro expansion paths, I don't think I can salvage my clever idea.

[Mark-Simulacrum]

@Nadrieril I did some experimentation, but wanted to confirm with you that we believe this one (unlike a previous similar sounding issue) is not a soundness bug, as we cannot actually remove the claimed-unreachable pattern in any case, right?

[Nadrieril]

@Mark-Simulacrum Agreed, this is not a soundness bug. The soundness-relevant code is independent of those Span shenanigans.