Rollup merge of #143619 - beetrees:varargs-named, r=jdonszelmann

`c_variadic`: Add future-incompatibility warning for `...` arguments without a pattern outside of `extern` blocks

This PR makes `...` arguments without a pattern in non-foreign functions (such as the argument in `unsafe extern "C" fn f(...) {}`) a future-compatibility warning; making this error would be consistent with how `unsafe extern "C" fn f(u32) {}` is handled. Allowing `...` arguments without a pattern in non-foreign functions is a source of confusion for programmers coming from C, where the `...` parameter is never named and instead calling `va_start` is required; disallowing `...` arguments without a pattern also improves the overall consistency of the Rust language by matching the treatment of other arguments without patterns. `...` arguments without a pattern in `extern` blocks (such as `unsafe extern "C" { fn f(...); }`) continue to compile without warnings after this PR, as they are already stable and heavily used (and don't cause the mentioned confusion as they are just being used in function declarations).

As all the syntax gating for `c_variadic` has been done post-expansion, this is technically a breaking change. In particular, code like this has compiled on stable since Rust 1.35.0:
```rust
#[cfg(any())] // Equivalent to the more recent #[cfg(false)]
unsafe extern "C" fn bar(_: u32, ...) {}
```
Since this is more or less a stability hole and a Crater run shows only the `binrw` crate is using this, I think it would be ok to break this. This will require a lang FCP.

The idea of rejecting `...` pre-expansion was first raised here #143546 (comment).

Tracking issue: #44930
cc `@folkertdev` `@workingjubilee`
r? `@joshtriplett`

Author: Zalathar

compiler/rustc_lint_defs/src/builtin.rs

@@ -144,6 +144,7 @@ declare_lint_pass! {
         UNUSED_UNSAFE,
         UNUSED_VARIABLES,
         USELESS_DEPRECATED,
+        VARARGS_WITHOUT_PATTERN,
         WARNINGS,
         // tidy-alphabetical-end
     ]
@@ -5295,3 +5296,50 @@ declare_lint! {
         report_in_deps: false,
     };
 }
+
+declare_lint! {
+    /// The `varargs_without_pattern` lint detects when `...` is used as an argument to a
+    /// non-foreign function without any pattern being specified.
+    ///
+    /// ### Example
+    ///
+    /// ```rust
+    /// // Using `...` in non-foreign function definitions is unstable, however stability is
+    /// // currently only checked after attributes are expanded, so using `#[cfg(false)]` here will
+    /// // allow this to compile on stable Rust.
+    /// #[cfg(false)]
+    /// fn foo(...) {
+    ///
+    /// }
+    /// ```
+    ///
+    /// {{produces}}
+    ///
+    /// ### Explanation
+    ///
+    /// Patterns are currently required for all non-`...` arguments in function definitions (with
+    /// some exceptions in the 2015 edition). Requiring `...` arguments to have patterns in
+    /// non-foreign function definitions makes the language more consistent, and removes a source of
+    /// confusion for the unstable C variadic feature. `...` arguments without a pattern are already
+    /// stable and widely used in foreign function definitions; this lint only affects non-foreign
+    /// function definitions.
+    ///
+    /// Using `...` (C varargs) in a non-foreign function definition is currently unstable. However,
+    /// stability checking for the `...` syntax in non-foreign function definitions is currently
+    /// implemented after attributes have been expanded, meaning that if the attribute removes the
+    /// use of the unstable syntax (e.g. `#[cfg(false)]`, or a procedural macro), the code will
+    /// compile on stable Rust; this is the only situation where this lint affects code that
+    /// compiles on stable Rust.
+    ///
+    /// This is a [future-incompatible] lint to transition this to a hard error in the future.
+    ///
+    /// [future-incompatible]: ../index.md#future-incompatible-lints
+    pub VARARGS_WITHOUT_PATTERN,
+    Warn,
+    "detects usage of `...` arguments without a pattern in non-foreign items",
+    @future_incompatible = FutureIncompatibleInfo {
+        reason: FutureIncompatibilityReason::FutureReleaseError,
+        reference: "issue #145544 <https://github.com/rust-lang/rust/issues/145544>",
+        report_in_deps: false,
+    };
+}

compiler/rustc_parse/messages.ftl

@@ -1004,6 +1004,9 @@ parse_use_if_else = use an `if-else` expression instead
 parse_use_let_not_auto = write `let` instead of `auto` to introduce a new variable
 parse_use_let_not_var = write `let` instead of `var` to introduce a new variable
 
+parse_varargs_without_pattern = missing pattern for `...` argument
+    .suggestion = name the argument, or use `_` to continue ignoring it
+
 parse_visibility_not_followed_by_item = visibility `{$vis}` is not followed by an item
     .label = the visibility
     .help = you likely meant to define an item, e.g., `{$vis} fn foo() {"{}"}`

compiler/rustc_parse/src/errors.rs

@@ -3645,3 +3645,10 @@ impl Subdiagnostic for HiddenUnicodeCodepointsDiagSub {
         }
     }
 }
+
+#[derive(LintDiagnostic)]
+#[diag(parse_varargs_without_pattern)]
+pub(crate) struct VarargsWithoutPattern {
+    #[suggestion(code = "_: ...", applicability = "machine-applicable")]
+    pub span: Span,
+}

compiler/rustc_parse/src/parser/attr.rs

@@ -201,7 +201,7 @@ impl<'a> Parser<'a> {
             AttrWrapper::empty(),
             true,
             false,
-            FnParseMode { req_name: |_| true, context: FnContext::Free, req_body: true },
+            FnParseMode { req_name: |_, _| true, context: FnContext::Free, req_body: true },
             ForceCollect::No,
         ) {
             Ok(Some(item)) => {

compiler/rustc_parse/src/parser/diagnostics.rs

@@ -45,6 +45,7 @@ use crate::errors::{
 };
 use crate::parser::FnContext;
 use crate::parser::attr::InnerAttrPolicy;
+use crate::parser::item::IsDotDotDot;
 use crate::{exp, fluent_generated as fluent};
 
 /// Creates a placeholder argument.
@@ -2284,7 +2285,7 @@ impl<'a> Parser<'a> {
             let maybe_emit_anon_params_note = |this: &mut Self, err: &mut Diag<'_>| {
                 let ed = this.token.span.with_neighbor(this.prev_token.span).edition();
                 if matches!(fn_parse_mode.context, crate::parser::item::FnContext::Trait)
-                    && (fn_parse_mode.req_name)(ed)
+                    && (fn_parse_mode.req_name)(ed, IsDotDotDot::No)
                 {
                     err.note("anonymous parameters are removed in the 2018 edition (see RFC 1685)");
                 }

compiler/rustc_parse/src/parser/item.rs

@@ -12,6 +12,7 @@ use rustc_ast::{
 use rustc_ast_pretty::pprust;
 use rustc_errors::codes::*;
 use rustc_errors::{Applicability, PResult, StashKey, struct_span_code_err};
+use rustc_session::lint::builtin::VARARGS_WITHOUT_PATTERN;
 use rustc_span::edit_distance::edit_distance;
 use rustc_span::edition::Edition;
 use rustc_span::{DUMMY_SP, ErrorGuaranteed, Ident, Span, Symbol, kw, source_map, sym};
@@ -119,7 +120,7 @@ impl<'a> Parser<'a> {
 impl<'a> Parser<'a> {
     pub fn parse_item(&mut self, force_collect: ForceCollect) -> PResult<'a, Option<Box<Item>>> {
         let fn_parse_mode =
-            FnParseMode { req_name: |_| true, context: FnContext::Free, req_body: true };
+            FnParseMode { req_name: |_, _| true, context: FnContext::Free, req_body: true };
         self.parse_item_(fn_parse_mode, force_collect).map(|i| i.map(Box::new))
     }
 
@@ -976,7 +977,7 @@ impl<'a> Parser<'a> {
         force_collect: ForceCollect,
     ) -> PResult<'a, Option<Option<Box<AssocItem>>>> {
         let fn_parse_mode =
-            FnParseMode { req_name: |_| true, context: FnContext::Impl, req_body: true };
+            FnParseMode { req_name: |_, _| true, context: FnContext::Impl, req_body: true };
         self.parse_assoc_item(fn_parse_mode, force_collect)
     }
 
@@ -985,7 +986,7 @@ impl<'a> Parser<'a> {
         force_collect: ForceCollect,
     ) -> PResult<'a, Option<Option<Box<AssocItem>>>> {
         let fn_parse_mode = FnParseMode {
-            req_name: |edition| edition >= Edition::Edition2018,
+            req_name: |edition, _| edition >= Edition::Edition2018,
             context: FnContext::Trait,
             req_body: false,
         };
@@ -1245,8 +1246,11 @@ impl<'a> Parser<'a> {
         &mut self,
         force_collect: ForceCollect,
     ) -> PResult<'a, Option<Option<Box<ForeignItem>>>> {
-        let fn_parse_mode =
-            FnParseMode { req_name: |_| true, context: FnContext::Free, req_body: false };
+        let fn_parse_mode = FnParseMode {
+            req_name: |_, is_dot_dot_dot| is_dot_dot_dot == IsDotDotDot::No,
+            context: FnContext::Free,
+            req_body: false,
+        };
         Ok(self.parse_item_(fn_parse_mode, force_collect)?.map(
             |Item { attrs, id, span, vis, kind, tokens }| {
                 let kind = match ForeignItemKind::try_from(kind) {
@@ -2133,7 +2137,7 @@ impl<'a> Parser<'a> {
                     Visibility { span: DUMMY_SP, kind: VisibilityKind::Inherited, tokens: None };
                 // We use `parse_fn` to get a span for the function
                 let fn_parse_mode =
-                    FnParseMode { req_name: |_| true, context: FnContext::Free, req_body: true };
+                    FnParseMode { req_name: |_, _| true, context: FnContext::Free, req_body: true };
                 match self.parse_fn(
                     &mut AttrVec::new(),
                     fn_parse_mode,
@@ -2366,8 +2370,16 @@ impl<'a> Parser<'a> {
 /// The function decides if, per-parameter `p`, `p` must have a pattern or just a type.
 ///
 /// This function pointer accepts an edition, because in edition 2015, trait declarations
-/// were allowed to omit parameter names. In 2018, they became required.
-type ReqName = fn(Edition) -> bool;
+/// were allowed to omit parameter names. In 2018, they became required. It also accepts an
+/// `IsDotDotDot` parameter, as `extern` function declarations and function pointer types are
+/// allowed to omit the name of the `...` but regular function items are not.
+type ReqName = fn(Edition, IsDotDotDot) -> bool;
+
+#[derive(Copy, Clone, PartialEq)]
+pub(crate) enum IsDotDotDot {
+    Yes,
+    No,
+}
 
 /// Parsing configuration for functions.
 ///
@@ -2400,6 +2412,9 @@ pub(crate) struct FnParseMode {
     ///     to true.
     ///   * The span is from Edition 2015. In particular, you can get a
     ///     2015 span inside a 2021 crate using macros.
+    ///
+    /// Or if `IsDotDotDot::Yes`, this function will also return `false` if the item being parsed
+    /// is inside an `extern` block.
     pub(super) req_name: ReqName,
     /// The context in which this function is parsed, used for diagnostics.
     /// This indicates the fn is a free function or method and so on.
@@ -3049,11 +3064,25 @@ impl<'a> Parser<'a> {
                 return Ok((res?, Trailing::No, UsePreAttrPos::No));
             }
 
-            let is_name_required = match this.token.kind {
-                token::DotDotDot => false,
-                _ => (fn_parse_mode.req_name)(
-                    this.token.span.with_neighbor(this.prev_token.span).edition(),
-                ),
+            let is_dot_dot_dot = if this.token.kind == token::DotDotDot {
+                IsDotDotDot::Yes
+            } else {
+                IsDotDotDot::No
+            };
+            let is_name_required = (fn_parse_mode.req_name)(
+                this.token.span.with_neighbor(this.prev_token.span).edition(),
+                is_dot_dot_dot,
+            );
+            let is_name_required = if is_name_required && is_dot_dot_dot == IsDotDotDot::Yes {
+                this.psess.buffer_lint(
+                    VARARGS_WITHOUT_PATTERN,
+                    this.token.span,
+                    ast::CRATE_NODE_ID,
+                    errors::VarargsWithoutPattern { span: this.token.span },
+                );
+                false
+            } else {
+                is_name_required
             };
             let (pat, ty) = if is_name_required || this.is_named_param() {
                 debug!("parse_param_general parse_pat (is_name_required:{})", is_name_required);

compiler/rustc_parse/src/parser/path.rs

@@ -404,7 +404,7 @@ impl<'a> Parser<'a> {
                         // Inside parenthesized type arguments, we want types only, not names.
                         let mode = FnParseMode {
                             context: FnContext::Free,
-                            req_name: |_| false,
+                            req_name: |_, _| false,
                             req_body: false,
                         };
                         let param = p.parse_param_general(&mode, false, false);

compiler/rustc_parse/src/parser/stmt.rs

@@ -154,7 +154,7 @@ impl<'a> Parser<'a> {
             attrs.clone(), // FIXME: unwanted clone of attrs
             false,
             true,
-            FnParseMode { req_name: |_| true, context: FnContext::Free, req_body: true },
+            FnParseMode { req_name: |_, _| true, context: FnContext::Free, req_body: true },
             force_collect,
         )? {
             self.mk_stmt(lo.to(item.span), StmtKind::Item(Box::new(item)))

compiler/rustc_parse/src/parser/ty.rs

@@ -824,7 +824,7 @@ impl<'a> Parser<'a> {
             self.recover_fn_ptr_with_generics(lo, &mut params, param_insertion_point)?;
         }
         let mode = crate::parser::item::FnParseMode {
-            req_name: |_| false,
+            req_name: |_, _| false,
             context: FnContext::Free,
             req_body: false,
         };
@@ -1380,7 +1380,8 @@ impl<'a> Parser<'a> {
         self.bump();
         let args_lo = self.token.span;
         let snapshot = self.create_snapshot_for_diagnostic();
-        let mode = FnParseMode { req_name: |_| false, context: FnContext::Free, req_body: false };
+        let mode =
+            FnParseMode { req_name: |_, _| false, context: FnContext::Free, req_body: false };
         match self.parse_fn_decl(&mode, AllowPlus::No, RecoverReturnSign::OnlyFatArrow) {
             Ok(decl) => {
                 self.dcx().emit_err(ExpectedFnPathFoundFnKeyword { fn_token_span });
@@ -1471,7 +1472,8 @@ impl<'a> Parser<'a> {
 
         // Parse `(T, U) -> R`.
         let inputs_lo = self.token.span;
-        let mode = FnParseMode { req_name: |_| false, context: FnContext::Free, req_body: false };
+        let mode =
+            FnParseMode { req_name: |_, _| false, context: FnContext::Free, req_body: false };
         let inputs: ThinVec<_> =
             self.parse_fn_params(&mode)?.into_iter().map(|input| input.ty).collect();
         let inputs_span = inputs_lo.to(self.prev_token.span);

tests/pretty/hir-fn-variadic.pp

@@ -16,12 +16,12 @@
 
 fn main() {
     fn g1(_: extern "C" fn(_: u8, va: ...)) { }
-    fn g2(_: extern "C" fn(_: u8, ...)) { }
+    fn g2(_: extern "C" fn(_: u8, _: ...)) { }
     fn g3(_: extern "C" fn(u8, va: ...)) { }
-    fn g4(_: extern "C" fn(u8, ...)) { }
+    fn g4(_: extern "C" fn(u8, _: ...)) { }
 
     fn g5(_: extern "C" fn(va: ...)) { }
-    fn g6(_: extern "C" fn(...)) { }
+    fn g6(_: extern "C" fn(_: ...)) { }
 
     {
         let _ =