Detect attempt to use var-args in closure

[estebank]

error: unexpected `...`
  --> $DIR/no-closure.rs:11:14
   |
LL |     let f = |...| {};
   |              ^^^ not a valid pattern
   |
   = note: only `extern "C"` and `extern "C-unwind"` functions may have a C variable argument list

error: unexpected `...`
  --> $DIR/no-closure.rs:16:17
   |
LL |     let f = |_: ...| {};
   |                 ^^^
   |
   = note: only `extern "C"` and `extern "C-unwind"` functions may have a C variable argument list

Fix #146489, when trying to use c-style var-args in a closure. We emit a more targeted message. We also silence inference errors when the pattern is PatKind::Err.

[rustbot]

r? @nnethercote

rustbot has assigned @nnethercote.
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

[rustbot]

Some changes occurred in need_type_info.rs

cc @lcnr

[lcnr]

hmm, surprised we don't have a way to silence a lint by passing in an ErrorGuaranteed. We could then do

        if let InferSourceKind::ClosureArg { kind: PatKind::Err(guar), .. } = kind {
            err.silence_via_error_guaranteed(guar);
        }
}

though maybe even better... change this function to return Result<Diag, ErrorGuaranteed> and avoid creating a diagnostic at all edit: seems annoying to do 😅

[estebank]

I guess the rationale for downgrade_to_delayed_bug itself not taking an ErrorGuarantee is because itself is kind of an error guarantee already :)

The guarantee is more critical for anything that could cause a binary to be emitted otherwise.

[lcnr]

existing, but what does "may not be nested inside another type" mean here 😅

[mu001999]

Could we just emit the error like using ... in normal fn:

error: `...` is not supported for non-extern functions
 --> src/lib.rs:3:8
  |
3 | fn foo(_: ...) {}
  |        ^^^^^^
  |
  = help: only `extern "C"` and `extern "C-unwind"` functions may have a C variable argument list

[estebank]

The reason this error triggers is because it is emitted by parse_ty_common, which was likely meant to catch uses like Vec<...>, but we didn't bother special casing the top-level case. It'll be hard to handle that in parse_ty_common because that doesn't know whether it is a top-level type being parsed, so it would have to be specially handled elsewhere. I'll look into it as a follow up.

Edit: scratch that. Curiosity got the best of me and fixed it quickly.

[lcnr]

r=me though still still doesn't seem as nice as it could be 🤔

could we explicitly state "closures must not be c-variadic?

[estebank]

still doesn't seem as nice as it could be 🤔

could we explicitly state "closures must not be c-variadic?

Being that specific will require more special casing in the parser. The new output is at least an improvement :)

error: unexpected `...`
  --> $DIR/no-closure.rs:11:14
   |
LL |     let f = |...| {};
   |              ^^^ not a valid pattern
   |
   = note: only `extern "C"` and `extern "C-unwind"` functions may have a C variable argument list

error: unexpected `...`
  --> $DIR/no-closure.rs:16:17
   |
LL |     let f = |_: ...| {};
   |                 ^^^
   |
   = note: only `extern "C"` and `extern "C-unwind"` functions may have a C variable argument list

[estebank]

@bors r=lcnr

[bors]

📌 Commit e9270e3 has been approved by lcnr

It is now in the queue for this repository.

[lcnr]

The PR looks good. For future PRs, I do feel like the change was substantial enough (especially by introducing a check at a new place) that I would have liked to look at it again myself