-
Notifications
You must be signed in to change notification settings - Fork 13.8k
Detect attempt to use var-args in closure #146581
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
``` error: unexpected `...` --> $DIR/varargs-in-closure-isnt-supported.rs:5:20 | LL | let mut lol = |...| (); | ^^^ not a valid pattern | = note: C-variadic type `...` is not allowed here ```
r? @nnethercote rustbot has assigned @nnethercote. Use |
Some changes occurred in need_type_info.rs cc @lcnr |
// We will have already emitted an error about this pattern. | ||
err.downgrade_to_delayed_bug(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
This comment has been minimized.
This comment has been minimized.
| | ||
= note: C-variadic type `...` is not allowed here | ||
|
||
error[E0743]: C-variadic type `...` may not be nested inside another type |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
existing, but what does "may not be nested inside another type" mean here 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
r=me though still still doesn't seem as nice as it could be 🤔 could we explicitly state "closures must not be |
When writing something like the expression `|_: ...| {}`, we now detect the `...` during parsing explicitly instead of relying on the detection in `parse_ty_common` so that we don't talk about "nested `...` are not supported". ``` error: unexpected `...` --> $DIR/no-closure.rs:6:35 | LL | const F: extern "C" fn(...) = |_: ...| {}; | ^^^ | = note: only `extern "C"` and `extern "C-unwind"` functions may have a C variable argument list ```
Being that specific will require more special casing in the parser. The new output is at least an improvement :)
|
@bors r=lcnr |
Detect attempt to use var-args in closure ``` 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 rust-lang#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`.
Rollup of 9 pull requests Successful merges: - #144871 (Stabilize `btree_entry_insert` feature) - #145181 (remove FIXME block from `has_significant_drop`, it never encounters inference variables) - #145838 (don't apply temporary lifetime extension rules to non-extended `super let`) - #146259 (Suggest removing Box::new instead of unboxing it) - #146410 (Iterator repeat: no infinite loop for `last` and `count`) - #146460 (Add tidy readme) - #146581 (Detect attempt to use var-args in closure) - #146588 (tests/run-make: Update list of statically linked musl targets) - #146647 (Move `#[rustc_coherence_is_core]` to the `crate_level` file) r? `@ghost` `@rustbot` modify labels: rollup
Detect attempt to use var-args in closure ``` 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 rust-lang#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`.
Rollup of 14 pull requests Successful merges: - #142807 (libtest: expose --fail-fast as an unstable command-line option) - #144871 (Stabilize `btree_entry_insert` feature) - #145071 (Update the minimum external LLVM to 20) - #145181 (remove FIXME block from `has_significant_drop`, it never encounters inference variables) - #145660 (initial implementation of the darwin_objc unstable feature) - #145838 (don't apply temporary lifetime extension rules to non-extended `super let`) - #146259 (Suggest removing Box::new instead of unboxing it) - #146410 (Iterator repeat: no infinite loop for `last` and `count`) - #146460 (Add tidy readme) - #146552 (StateTransform: Do not renumber resume local.) - #146564 (Remove Rvalue::Len again.) - #146581 (Detect attempt to use var-args in closure) - #146588 (tests/run-make: Update list of statically linked musl targets) - #146631 (cg_llvm: Replace some DIBuilder wrappers with LLVM-C API bindings (part 3)) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of #146581 - estebank:issue-146489, r=lcnr Detect attempt to use var-args in closure ``` 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`.
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 |
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
.