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

Author: beetrees

compiler/rustc_lint_defs/src/builtin.rs

@@ -142,6 +142,7 @@ declare_lint_pass! {
         UNUSED_UNSAFE,
         UNUSED_VARIABLES,
         USELESS_DEPRECATED,
+        VARARGS_WITHOUT_PATTERN,
         WARNINGS,
         // tidy-alphabetical-end
     ]
@@ -5196,3 +5197,50 @@ declare_lint! {
     Warn,
     r#"detects when a function annotated with `#[inline(always)]` and `#[target_feature(enable = "..")]` is inlined into a caller without the required target feature"#,
 }
+
+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

@@ -1005,6 +1005,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

@@ -3653,3 +3653,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

@@ -46,6 +46,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.
@@ -2285,7 +2286,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

@@ -10,6 +10,7 @@ use rustc_ast::{self as 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};
@@ -117,7 +118,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))
     }
 
@@ -977,7 +978,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)
     }
 
@@ -986,7 +987,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) {
@@ -2121,7 +2125,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,
@@ -2354,8 +2358,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.
 ///
@@ -2388,6 +2400,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.
@@ -3037,11 +3052,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

@@ -803,7 +803,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,
         };
@@ -1364,7 +1364,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 });
@@ -1455,7 +1456,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 _ =