[lapla-cogito]

Closes #165365

[rustbot]

r? @adwinwhite

rustbot has assigned @adwinwhite.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

• Owners of files modified in this PR: compiler
• compiler expanded to 77 candidates
• Random selection from 22 candidates

[dianne]

this might need to account for the constructor(s) used in the pattern? it e.g. still produces false positives when the success and error types of Result are the same, such as for Err([]) matching on Result<Vec<E>, Vec<E>>

or for a more contrived example where the pattern itself matters,

enum Contrived<'a> {
    Layer(&'a Contrived<'a>),
    Value(Vec<E>),
}

impl Deref for Contrived<'_> {
    type Target = [E];
    fn deref(&self) -> &[E] {
        match self {
            Contrived::Layer(peeled) => peeled.deref(),
            Contrived::Value(value) => value.deref(),
        }
    }
}

fn contrived(opt: Option<Contrived<'_>>) {
    match opt {
        // suggests `.as_deref()`, but that causes a type mismatch:
        Some(Contrived::Layer([])) => {}
        _ => {}
    }
}

View changes since the review

[lapla-cogito]

Thanks! Now only suggest when the slice pattern is the payload of Some or Ok itself.

[dianne]

this traversal looks pretty complicated relative to what we'd be gaining from it. a few thoughts:

• maybe it could help to use a hir parent iterator to start at the slice pattern and go up to the top? that way it wouldn't have to search each or-pattern alternative
• instead of tracking state with the AsDerefPatPos enum, maybe it would be clearer to have a helper that peels off &s/guards/or-patterns/etc.? with a hir parent iterator I imagine you could check that doing that once gets you to an Ok/Some and doing it again gets you to the top of the pattern
• the cases that continue searching with AsDerefPatPos::Rejected look like they could just return false instead

View changes since the review

[lapla-cogito]

It makes sense. Thanks.

[dianne]

probably rare enough that it's not worth accounting for, but just in case it's easy, I feel obliged to note that Some { 0: [] } and Ok { 0: [] } are equivalent to Some([]) and Ok([])

View changes since the review