Move allow_c_varadic logic to ast_validation.

Author: Centril

src/librustc_parse/parser/item.rs

@@ -1727,8 +1727,6 @@ impl<'a> Parser<'a> {
 pub(super) struct ParamCfg {
     /// Is `self` is allowed as the first parameter?
     pub is_self_allowed: bool,
-    /// Is `...` allowed as the tail of the parameter list?
-    pub allow_c_variadic: bool,
     /// `is_name_required` decides if, per-parameter,
     /// the parameter must have a pattern or just a type.
     pub is_name_required: fn(&token::Token) -> bool,
@@ -1744,16 +1742,8 @@ impl<'a> Parser<'a> {
         attrs: Vec<Attribute>,
         header: FnHeader,
     ) -> PResult<'a, Option<P<Item>>> {
-        let is_c_abi = match header.ext {
-            ast::Extern::None => false,
-            ast::Extern::Implicit => true,
-            ast::Extern::Explicit(abi) => abi.symbol_unescaped == sym::C,
-        };
         let (ident, decl, generics) = self.parse_fn_sig(ParamCfg {
             is_self_allowed: false,
-            // FIXME: Parsing should not depend on ABI or unsafety and
-            // the variadic parameter should always be parsed.
-            allow_c_variadic: is_c_abi && header.unsafety == Unsafety::Unsafe,
             is_name_required: |_| true,
         })?;
         let (inner_attrs, body) = self.parse_inner_attrs_and_block()?;
@@ -1772,7 +1762,6 @@ impl<'a> Parser<'a> {
         self.expect_keyword(kw::Fn)?;
         let (ident, decl, generics) = self.parse_fn_sig(ParamCfg {
             is_self_allowed: false,
-            allow_c_variadic: true,
             is_name_required: |_| true,
         })?;
         let span = lo.to(self.token.span);
@@ -1797,7 +1786,6 @@ impl<'a> Parser<'a> {
         let header = self.parse_fn_front_matter()?;
         let (ident, decl, generics) = self.parse_fn_sig(ParamCfg {
             is_self_allowed: true,
-            allow_c_variadic: false,
             is_name_required,
         })?;
         let sig = FnSig { header, decl };
@@ -1993,12 +1981,12 @@ impl<'a> Parser<'a> {
             }
 
             self.eat_incorrect_doc_comment_for_param_type();
-            (pat, self.parse_ty_for_param(cfg.allow_c_variadic)?)
+            (pat, self.parse_ty_for_param()?)
         } else {
             debug!("parse_param_general ident_to_pat");
             let parser_snapshot_before_ty = self.clone();
             self.eat_incorrect_doc_comment_for_param_type();
-            let mut ty = self.parse_ty_for_param(cfg.allow_c_variadic);
+            let mut ty = self.parse_ty_for_param();
             if ty.is_ok() && self.token != token::Comma &&
                self.token != token::CloseDelim(token::Paren) {
                 // This wasn't actually a type, but a pattern looking like a type,

src/librustc_parse/parser/ty.rs

@@ -33,8 +33,8 @@ impl<'a> Parser<'a> {
     /// Parse a type suitable for a function or function pointer parameter.
     /// The difference from `parse_ty` is that this version allows `...`
     /// (`CVarArgs`) at the top level of the the type.
-    pub(super) fn parse_ty_for_param(&mut self, allow_c_variadic: bool) -> PResult<'a, P<Ty>> {
-        self.parse_ty_common(true, true, allow_c_variadic)
+    pub(super) fn parse_ty_for_param(&mut self) -> PResult<'a, P<Ty>> {
+        self.parse_ty_common(true, true, true)
     }
 
     /// Parses a type in restricted contexts where `+` is not permitted.
@@ -306,7 +306,6 @@ impl<'a> Parser<'a> {
         self.expect_keyword(kw::Fn)?;
         let cfg = ParamCfg {
             is_self_allowed: false,
-            allow_c_variadic: true,
             is_name_required: |_| false,
         };
         let decl = self.parse_fn_decl(cfg, false)?;

src/librustc_passes/ast_validation.rs

@@ -306,6 +306,19 @@ impl<'a> AstValidator<'a> {
             .struct_span_err(span, "bounds on associated `type`s in `impl`s have no effect")
             .emit();
     }
+
+    fn check_c_varadic_type(&self, decl: &FnDecl) {
+        for Param { ty, span, .. } in &decl.inputs {
+            if let TyKind::CVarArgs = ty.kind {
+                self.err_handler()
+                    .struct_span_err(
+                        *span,
+                        "only foreign or `unsafe extern \"C\" functions may be C-variadic",
+                    )
+                    .emit();
+            }
+        }
+    }
 }
 
 enum GenericPosition {
@@ -554,6 +567,12 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
                         }
                     }
                 }
+                // Reject C-varadic type unless the function is `unsafe extern "C"` semantically.
+                match sig.header.ext {
+                    Extern::Explicit(StrLit { symbol_unescaped: sym::C, .. }) |
+                    Extern::Implicit if sig.header.unsafety == Unsafety::Unsafe => {}
+                    _ => self.check_c_varadic_type(&sig.decl),
+                }
             }
             ItemKind::ForeignMod(..) => {
                 self.invalid_visibility(
@@ -795,6 +814,13 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
         self.check_defaultness(ti.span, ti.defaultness);
         visit::walk_trait_item(self, ti);
     }
+
+    fn visit_assoc_item(&mut self, item: &'a AssocItem) {
+        if let AssocItemKind::Method(sig, _) = &item.kind {
+            self.check_c_varadic_type(&sig.decl);
+        }
+        visit::walk_assoc_item(self, item);
+    }
 }
 
 pub fn check_crate(session: &Session, krate: &Crate, lints: &mut lint::LintBuffer) -> bool {

src/test/ui/invalid/invalid-variadic-function.rs

@@ -0,0 +1,26 @@
+#![feature(c_variadic)]
+
+fn main() {}
+
+fn f1(x: isize, ...) {}
+//~^ ERROR: only foreign or `unsafe extern "C" functions may be C-variadic
+
+extern "C" fn f2(x: isize, ...) {}
+//~^ ERROR: only foreign or `unsafe extern "C" functions may be C-variadic
+
+extern fn f3(x: isize, ...) {}
+//~^ ERROR: only foreign or `unsafe extern "C" functions may be C-variadic
+
+struct X;
+
+impl X {
+    fn f4(x: isize, ...) {}
+    //~^ ERROR: only foreign or `unsafe extern "C" functions may be C-variadic
+}
+
+trait T {
+    fn f5(x: isize, ...) {}
+    //~^ ERROR: only foreign or `unsafe extern "C" functions may be C-variadic
+    fn f6(x: isize, ...);
+    //~^ ERROR: only foreign or `unsafe extern "C" functions may be C-variadic
+}

src/test/ui/invalid/invalid-variadic-function.stderr

@@ -0,0 +1,38 @@
+error: only foreign or `unsafe extern "C" functions may be C-variadic
+  --> $DIR/variadic-ffi-semantic-restrictions.rs:5:17
+   |
+LL | fn f1(x: isize, ...) {}
+   |                 ^^^^
+
+error: only foreign or `unsafe extern "C" functions may be C-variadic
+  --> $DIR/variadic-ffi-semantic-restrictions.rs:8:28
+   |
+LL | extern "C" fn f2(x: isize, ...) {}
+   |                            ^^^^
+
+error: only foreign or `unsafe extern "C" functions may be C-variadic
+  --> $DIR/variadic-ffi-semantic-restrictions.rs:11:24
+   |
+LL | extern fn f3(x: isize, ...) {}
+   |                        ^^^^
+
+error: only foreign or `unsafe extern "C" functions may be C-variadic
+  --> $DIR/variadic-ffi-semantic-restrictions.rs:17:21
+   |
+LL |     fn f4(x: isize, ...) {}
+   |                     ^^^^
+
+error: only foreign or `unsafe extern "C" functions may be C-variadic
+  --> $DIR/variadic-ffi-semantic-restrictions.rs:22:21
+   |
+LL |     fn f5(x: isize, ...) {}
+   |                     ^^^^
+
+error: only foreign or `unsafe extern "C" functions may be C-variadic
+  --> $DIR/variadic-ffi-semantic-restrictions.rs:24:21
+   |
+LL |     fn f6(x: isize, ...);
+   |                     ^^^^
+
+error: aborting due to 6 previous errors
+

src/test/ui/parser/variadic-ffi-3.rs

@@ -0,0 +1,25 @@
+// check-pass
+
+fn main() {}
+
+#[cfg(FALSE)]
+fn f1(x: isize, ...) {}
+
+#[cfg(FALSE)]
+extern "C" fn f2(x: isize, ...) {}
+
+#[cfg(FALSE)]
+extern fn f3(x: isize, ...) {}
+
+struct X;
+
+#[cfg(FALSE)]
+impl X {
+    fn f4(x: isize, ...) {}
+}
+
+#[cfg(FALSE)]
+trait T {
+    fn f5(x: isize, ...) {}
+    fn f6(x: isize, ...);
+}