From 0d41d0fe1423b4d21d6a9a2778419d399da1a13a Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Mon, 2 Dec 2019 02:38:33 +0100 Subject: [PATCH] Move `allow_c_varadic` logic to `ast_validation`. --- src/librustc_parse/parser/item.rs | 16 +------- src/librustc_parse/parser/ty.rs | 5 +-- src/librustc_passes/ast_validation.rs | 26 +++++++++++++ .../ui/invalid/invalid-variadic-function.rs | 3 -- .../invalid/invalid-variadic-function.stderr | 15 -------- src/test/ui/parser/variadic-ffi-3.rs | 5 --- src/test/ui/parser/variadic-ffi-3.stderr | 9 ----- src/test/ui/parser/variadic-ffi-4.rs | 5 --- src/test/ui/parser/variadic-ffi-4.stderr | 9 ----- .../variadic-ffi-semantic-restrictions.rs | 26 +++++++++++++ .../variadic-ffi-semantic-restrictions.stderr | 38 +++++++++++++++++++ .../ui/parser/variadic-ffi-syntactic-pass.rs | 25 ++++++++++++ 12 files changed, 119 insertions(+), 63 deletions(-) delete mode 100644 src/test/ui/invalid/invalid-variadic-function.rs delete mode 100644 src/test/ui/invalid/invalid-variadic-function.stderr delete mode 100644 src/test/ui/parser/variadic-ffi-3.rs delete mode 100644 src/test/ui/parser/variadic-ffi-3.stderr delete mode 100644 src/test/ui/parser/variadic-ffi-4.rs delete mode 100644 src/test/ui/parser/variadic-ffi-4.stderr create mode 100644 src/test/ui/parser/variadic-ffi-semantic-restrictions.rs create mode 100644 src/test/ui/parser/variadic-ffi-semantic-restrictions.stderr create mode 100644 src/test/ui/parser/variadic-ffi-syntactic-pass.rs diff --git a/src/librustc_parse/parser/item.rs b/src/librustc_parse/parser/item.rs index 0126297a358..d4b62e8ebba 100644 --- a/src/librustc_parse/parser/item.rs +++ b/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, header: FnHeader, ) -> PResult<'a, Option>> { - 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, diff --git a/src/librustc_parse/parser/ty.rs b/src/librustc_parse/parser/ty.rs index 1dffa6c94a8..3ab290d1cbb 100644 --- a/src/librustc_parse/parser/ty.rs +++ b/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> { - self.parse_ty_common(true, true, allow_c_variadic) + pub(super) fn parse_ty_for_param(&mut self) -> PResult<'a, P> { + 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)?; diff --git a/src/librustc_passes/ast_validation.rs b/src/librustc_passes/ast_validation.rs index ad6c99494a6..a26c991c9cf 100644 --- a/src/librustc_passes/ast_validation.rs +++ b/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 { diff --git a/src/test/ui/invalid/invalid-variadic-function.rs b/src/test/ui/invalid/invalid-variadic-function.rs deleted file mode 100644 index 8d23f0e4770..00000000000 --- a/src/test/ui/invalid/invalid-variadic-function.rs +++ /dev/null @@ -1,3 +0,0 @@ -extern "C" fn foo(x: u8, ...); -//~^ ERROR only foreign functions are allowed to be C-variadic -//~| ERROR expected one of `->`, `where`, or `{`, found `;` diff --git a/src/test/ui/invalid/invalid-variadic-function.stderr b/src/test/ui/invalid/invalid-variadic-function.stderr deleted file mode 100644 index 7e58b17e7db..00000000000 --- a/src/test/ui/invalid/invalid-variadic-function.stderr +++ /dev/null @@ -1,15 +0,0 @@ -error[E0743]: only foreign functions are allowed to be C-variadic - --> $DIR/invalid-variadic-function.rs:1:26 - | -LL | extern "C" fn foo(x: u8, ...); - | ^^^ - -error: expected one of `->`, `where`, or `{`, found `;` - --> $DIR/invalid-variadic-function.rs:1:30 - | -LL | extern "C" fn foo(x: u8, ...); - | ^ expected one of `->`, `where`, or `{` - -error: aborting due to 2 previous errors - -For more information about this error, try `rustc --explain E0743`. diff --git a/src/test/ui/parser/variadic-ffi-3.rs b/src/test/ui/parser/variadic-ffi-3.rs deleted file mode 100644 index ce83cc87abe..00000000000 --- a/src/test/ui/parser/variadic-ffi-3.rs +++ /dev/null @@ -1,5 +0,0 @@ -fn foo(x: isize, ...) { - //~^ ERROR: only foreign functions are allowed to be C-variadic -} - -fn main() {} diff --git a/src/test/ui/parser/variadic-ffi-3.stderr b/src/test/ui/parser/variadic-ffi-3.stderr deleted file mode 100644 index aeeebdb9914..00000000000 --- a/src/test/ui/parser/variadic-ffi-3.stderr +++ /dev/null @@ -1,9 +0,0 @@ -error[E0743]: only foreign functions are allowed to be C-variadic - --> $DIR/variadic-ffi-3.rs:1:18 - | -LL | fn foo(x: isize, ...) { - | ^^^ - -error: aborting due to previous error - -For more information about this error, try `rustc --explain E0743`. diff --git a/src/test/ui/parser/variadic-ffi-4.rs b/src/test/ui/parser/variadic-ffi-4.rs deleted file mode 100644 index 5f8b3f8f539..00000000000 --- a/src/test/ui/parser/variadic-ffi-4.rs +++ /dev/null @@ -1,5 +0,0 @@ -extern "C" fn foo(x: isize, ...) { - //~^ ERROR: only foreign functions are allowed to be C-variadic -} - -fn main() {} diff --git a/src/test/ui/parser/variadic-ffi-4.stderr b/src/test/ui/parser/variadic-ffi-4.stderr deleted file mode 100644 index da83276c72d..00000000000 --- a/src/test/ui/parser/variadic-ffi-4.stderr +++ /dev/null @@ -1,9 +0,0 @@ -error[E0743]: only foreign functions are allowed to be C-variadic - --> $DIR/variadic-ffi-4.rs:1:29 - | -LL | extern "C" fn foo(x: isize, ...) { - | ^^^ - -error: aborting due to previous error - -For more information about this error, try `rustc --explain E0743`. diff --git a/src/test/ui/parser/variadic-ffi-semantic-restrictions.rs b/src/test/ui/parser/variadic-ffi-semantic-restrictions.rs new file mode 100644 index 00000000000..57086bca2f4 --- /dev/null +++ b/src/test/ui/parser/variadic-ffi-semantic-restrictions.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 +} diff --git a/src/test/ui/parser/variadic-ffi-semantic-restrictions.stderr b/src/test/ui/parser/variadic-ffi-semantic-restrictions.stderr new file mode 100644 index 00000000000..69244d92ee3 --- /dev/null +++ b/src/test/ui/parser/variadic-ffi-semantic-restrictions.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 + diff --git a/src/test/ui/parser/variadic-ffi-syntactic-pass.rs b/src/test/ui/parser/variadic-ffi-syntactic-pass.rs new file mode 100644 index 00000000000..f8fcce6ba73 --- /dev/null +++ b/src/test/ui/parser/variadic-ffi-syntactic-pass.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, ...); +}