regression: Panic instead of validation error when checking binding compatibility

[sagudev]

While updating servo from f25e07b to c0e7c1e in servo/servo#32827, I noticed new crash (also reproducible on latest firefox nightly) on: webgpu:api,validation,encoding,programmable,pipeline_bind_group_compat:bgl_binding_mismatch:* (src of the test: https://github.com/gpuweb/cts/blob/50b6e7a7435e8d1a973cbf67347938ce05188df0/src/webgpu/api/validation/encoding/programmable/pipeline_bind_group_compat.spec.ts#L636)

Description
On f25e07b there is validation error:

Validation {
    source: ContextError {
        string: "ComputePass::end",
        cause: ComputePassError {
            scope: Dispatch {
                indirect: false,
            },
            inner: Dispatch(
                IncompatibleBindGroup(
                    IncompatibleBindGroupError {
                        index: 0,
                        pipeline: ResourceErrorIdent {
                            type: "ComputePipeline",
                            label: "",
                        },
                        diff: [
                            "Should be compatible an with an explicit BindGroupLayout with '' label",
                            "Assigned explicit BindGroupLayout with '' label",
                            "Entry 3 not found in assigned bind group layout",
                            "Entry 2 not found in expected bind group layout",
                        ],
                    },
                ),
            ),
        },
        label_key: "label",
        label: "",
    },
    description: "Validation Error\n\nCaused by:\n    In ComputePass::end\n    In a dispatch command, indirect:false\n    In a dispatch command, indirect:false\n    Bind group at index 0 is incompatible with the current set ComputePipeline with '' label\n",
}

but c0e7c1e (also on current trunk: 205f1e3) panics on

wgpu/wgpu-core/src/command/bind.rs

Line 187 in 205f1e3

inner: MultiError::new(errors.drain(..)).unwrap(),

:

Hello, world!
thread 'main' panicked at /home/user/.cargo/git/checkouts/wgpu-53e70f8674b08dd4/c0e7c1e/wgpu-core/src/command/bind.rs:187:70:
called `Option::unwrap()` on a `None` value
stack backtrace:
   0: rust_begin_unwind
             at /rustc/9b00956e56009bab2aa15d7bff10916599e3d6d6/library/std/src/panicking.rs:645:5
   1: core::panicking::panic_fmt
             at /rustc/9b00956e56009bab2aa15d7bff10916599e3d6d6/library/core/src/panicking.rs:72:14
   2: core::panicking::panic
             at /rustc/9b00956e56009bab2aa15d7bff10916599e3d6d6/library/core/src/panicking.rs:145:5
   3: core::option::unwrap_failed
             at /rustc/9b00956e56009bab2aa15d7bff10916599e3d6d6/library/core/src/option.rs:1985:5
   4: core::option::Option<T>::unwrap
             at /rustc/9b00956e56009bab2aa15d7bff10916599e3d6d6/library/core/src/option.rs:933:21
   5: wgpu_core::command::bind::compat::Entry<A>::check
             at /home/user/.cargo/git/checkouts/wgpu-53e70f8674b08dd4/c0e7c1e/wgpu-core/src/command/bind.rs:187:36
   6: wgpu_core::command::bind::compat::BoundBindGroupLayouts<A>::get_invalid
             at /home/user/.cargo/git/checkouts/wgpu-53e70f8674b08dd4/c0e7c1e/wgpu-core/src/command/bind.rs:258:17
   7: wgpu_core::command::bind::Binder<A>::check_compatibility
             at /home/user/.cargo/git/checkouts/wgpu-53e70f8674b08dd4/c0e7c1e/wgpu-core/src/command/bind.rs:432:9
   8: wgpu_core::command::compute::State<A>::is_ready
             at /home/user/.cargo/git/checkouts/wgpu-53e70f8674b08dd4/c0e7c1e/wgpu-core/src/command/compute.rs:238:13
   9: wgpu_core::command::compute::dispatch
             at /home/user/.cargo/git/checkouts/wgpu-53e70f8674b08dd4/c0e7c1e/wgpu-core/src/command/compute.rs:847:5
  10: wgpu_core::command::compute::<impl wgpu_core::global::Global>::compute_pass_end_impl
             at /home/user/.cargo/git/checkouts/wgpu-53e70f8674b08dd4/c0e7c1e/wgpu-core/src/command/compute.rs:586:21
  11: wgpu_core::command::compute::<impl wgpu_core::global::Global>::compute_pass_end
             at /home/user/.cargo/git/checkouts/wgpu-53e70f8674b08dd4/c0e7c1e/wgpu-core/src/command/compute.rs:368:9
  12: <wgpu_core::command::compute::ComputePass<A> as wgpu_core::command::dyn_compute_pass::DynComputePass>::end
             at /home/user/.cargo/git/checkouts/wgpu-53e70f8674b08dd4/c0e7c1e/wgpu-core/src/command/dyn_compute_pass.rs:172:9
  13: <wgpu::backend::wgpu_core::ContextWgpuCore as wgpu::context::Context>::compute_pass_end
             at /home/user/.cargo/git/checkouts/wgpu-53e70f8674b08dd4/c0e7c1e/wgpu/src/backend/wgpu_core.rs:2587:29
  14: <T as wgpu::context::DynContext>::compute_pass_end
             at /home/user/.cargo/git/checkouts/wgpu-53e70f8674b08dd4/c0e7c1e/wgpu/src/context.rs:3306:9
  15: <wgpu::api::compute_pass::ComputePassInner as core::ops::drop::Drop>::drop
             at /home/user/.cargo/git/checkouts/wgpu-53e70f8674b08dd4/c0e7c1e/wgpu/src/api/compute_pass.rs:215:13
  16: core::ptr::drop_in_place<wgpu::api::compute_pass::ComputePassInner>
             at /rustc/9b00956e56009bab2aa15d7bff10916599e3d6d6/library/core/src/ptr/mod.rs:515:1
  17: core::ptr::drop_in_place<wgpu::api::compute_pass::ComputePass>
             at /rustc/9b00956e56009bab2aa15d7bff10916599e3d6d6/library/core/src/ptr/mod.rs:515:1
  18: wgpu_problem::run::{{closure}}
             at ./src/main.rs:190:5
  19: pollster::block_on
             at /home/user/.cargo/registry/src/index.crates.io-6f17d22bba15001f/pollster-0.3.0/src/lib.rs:128:15
  20: wgpu_problem::main
             at ./src/main.rs:13:5
  21: core::ops::function::FnOnce::call_once
             at /rustc/9b00956e56009bab2aa15d7bff10916599e3d6d6/library/core/src/ops/function.rs:250:5

Repro steps
I was able to reproduce what CTS does by tracing wgpu functions that are called by servo; repro is available here: https://github.com/sagudev/wgpu-problem/tree/empty-bind-error

[sagudev]

I was able to narrow it down to 4a19ac2 (first bad commit), cc @teoxoy.

[sagudev]

The problem is that there is no if a_entry.binding != e_entry.binding { check anymore.

[sagudev]

I have a fix in #6012, but maybe it would be worth to fallback to dummy unknown error instead of panicking unwrap?

[teoxoy]

Ideally we'd show the same ExtraExpected/ExtraAssigned error for this case, that was the intent when I removed if a_entry.binding != e_entry.binding { but I missed that having holes in the BGL is fine.

[teoxoy]

It's also confusing that we have the binding in 2 places, we should just iterate over the values.