Skip to content

Rollup of 4 pull requests #68768

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

Closed
wants to merge 9 commits into from
4 changes: 2 additions & 2 deletions src/libcore/option.rs
Original file line number Diff line number Diff line change
@@ -331,12 +331,12 @@ impl<T> Option<T> {
///
/// ```
/// let x = Some("value");
/// assert_eq!(x.expect("the world is ending"), "value");
/// assert_eq!(x.expect("fruits are healthy"), "value");
/// ```
///
/// ```{.should_panic}
/// let x: Option<&str> = None;
/// x.expect("the world is ending"); // panics with `the world is ending`
/// x.expect("fruits are healthy"); // panics with `fruits are healthy`
/// ```
#[inline]
#[track_caller]
44 changes: 36 additions & 8 deletions src/librustc_ast_passes/ast_validation.rs
Original file line number Diff line number Diff line change
@@ -23,6 +23,12 @@ use syntax::expand::is_proc_macro_attr;
use syntax::visit::{self, Visitor};
use syntax::walk_list;

/// Is `self` allowed semantically as the first parameter in an `FnDecl`?
enum SelfSemantic {
Yes,
No,
}

/// A syntactic context that disallows certain kinds of bounds (e.g., `?Trait` or `?const Trait`).
#[derive(Clone, Copy)]
enum BoundContext {
@@ -302,7 +308,13 @@ impl<'a> AstValidator<'a> {
}
}

fn check_fn_decl(&self, fn_decl: &FnDecl) {
fn check_fn_decl(&self, fn_decl: &FnDecl, self_semantic: SelfSemantic) {
self.check_decl_cvaradic_pos(fn_decl);
self.check_decl_attrs(fn_decl);
self.check_decl_self_param(fn_decl, self_semantic);
}

fn check_decl_cvaradic_pos(&self, fn_decl: &FnDecl) {
match &*fn_decl.inputs {
[Param { ty, span, .. }] => {
if let TyKind::CVarArgs = ty.kind {
@@ -324,7 +336,9 @@ impl<'a> AstValidator<'a> {
}
_ => {}
}
}

fn check_decl_attrs(&self, fn_decl: &FnDecl) {
fn_decl
.inputs
.iter()
@@ -352,6 +366,21 @@ impl<'a> AstValidator<'a> {
});
}

fn check_decl_self_param(&self, fn_decl: &FnDecl, self_semantic: SelfSemantic) {
if let (SelfSemantic::No, [param, ..]) = (self_semantic, &*fn_decl.inputs) {
if param.is_self() {
self.err_handler()
.struct_span_err(
param.span,
"`self` parameter is only allowed in associated functions",
)
.span_label(param.span, "not semantically valid as function parameter")
.note("associated functions are those in `impl` or `trait` definitions")
.emit();
}
}
}

fn check_defaultness(&self, span: Span, defaultness: Defaultness) {
if let Defaultness::Default = defaultness {
self.err_handler()
@@ -504,7 +533,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
fn visit_expr(&mut self, expr: &'a Expr) {
match &expr.kind {
ExprKind::Closure(_, _, _, fn_decl, _, _) => {
self.check_fn_decl(fn_decl);
self.check_fn_decl(fn_decl, SelfSemantic::No);
}
ExprKind::InlineAsm(..) if !self.session.target.target.options.allow_asm => {
struct_span_err!(
@@ -524,7 +553,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
fn visit_ty(&mut self, ty: &'a Ty) {
match ty.kind {
TyKind::BareFn(ref bfty) => {
self.check_fn_decl(&bfty.decl);
self.check_fn_decl(&bfty.decl, SelfSemantic::No);
Self::check_decl_no_pat(&bfty.decl, |span, _| {
struct_span_err!(
self.session,
@@ -685,7 +714,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
}
ItemKind::Fn(ref sig, ref generics, _) => {
self.visit_fn_header(&sig.header);
self.check_fn_decl(&sig.decl);
self.check_fn_decl(&sig.decl, SelfSemantic::No);
// We currently do not permit const generics in `const fn`, as
// this is tantamount to allowing compile-time dependent typing.
if sig.header.constness.node == Constness::Const {
@@ -793,7 +822,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
fn visit_foreign_item(&mut self, fi: &'a ForeignItem) {
match fi.kind {
ForeignItemKind::Fn(ref decl, _) => {
self.check_fn_decl(decl);
self.check_fn_decl(decl, SelfSemantic::No);
Self::check_decl_no_pat(decl, |span, _| {
struct_span_err!(
self.session,
@@ -987,9 +1016,8 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
AssocItemKind::Const(_, body) => {
self.check_impl_item_provided(ii.span, body, "constant", " = <expr>;");
}
AssocItemKind::Fn(sig, body) => {
AssocItemKind::Fn(_, body) => {
self.check_impl_item_provided(ii.span, body, "function", " { <body> }");
self.check_fn_decl(&sig.decl);
}
AssocItemKind::TyAlias(bounds, body) => {
self.check_impl_item_provided(ii.span, body, "type", " = <type>;");
@@ -1005,7 +1033,6 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
self.check_defaultness(ti.span, ti.defaultness);

if let AssocItemKind::Fn(sig, block) = &ti.kind {
self.check_fn_decl(&sig.decl);
self.check_trait_fn_not_async(ti.span, sig.header.asyncness.node);
self.check_trait_fn_not_const(sig.header.constness);
if block.is_none() {
@@ -1035,6 +1062,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> {

fn visit_assoc_item(&mut self, item: &'a AssocItem) {
if let AssocItemKind::Fn(sig, _) = &item.kind {
self.check_fn_decl(&sig.decl, SelfSemantic::Yes);
self.check_c_varadic_type(&sig.decl);
}
visit::walk_assoc_item(self, item);
25 changes: 7 additions & 18 deletions src/librustc_parse/parser/diagnostics.rs
Original file line number Diff line number Diff line change
@@ -1336,8 +1336,7 @@ impl<'a> Parser<'a> {
err: &mut DiagnosticBuilder<'_>,
pat: P<ast::Pat>,
require_name: bool,
is_self_allowed: bool,
is_trait_item: bool,
first_param: bool,
) -> Option<Ident> {
// If we find a pattern followed by an identifier, it could be an (incorrect)
// C-style parameter declaration.
@@ -1357,13 +1356,12 @@ impl<'a> Parser<'a> {
return Some(ident);
} else if let PatKind::Ident(_, ident, _) = pat.kind {
if require_name
&& (is_trait_item
|| self.token == token::Comma
&& (self.token == token::Comma
|| self.token == token::Lt
|| self.token == token::CloseDelim(token::Paren))
{
// `fn foo(a, b) {}`, `fn foo(a<x>, b<y>) {}` or `fn foo(usize, usize) {}`
if is_self_allowed {
if first_param {
err.span_suggestion(
pat.span,
"if this is a `self` type, give it a parameter name",
@@ -1420,21 +1418,12 @@ impl<'a> Parser<'a> {
Ok((pat, ty))
}

pub(super) fn recover_bad_self_param(
&mut self,
mut param: ast::Param,
is_trait_item: bool,
) -> PResult<'a, ast::Param> {
pub(super) fn recover_bad_self_param(&mut self, mut param: Param) -> PResult<'a, Param> {
let sp = param.pat.span;
param.ty.kind = TyKind::Err;
let mut err = self.struct_span_err(sp, "unexpected `self` parameter in function");
if is_trait_item {
err.span_label(sp, "must be the first associated function parameter");
} else {
err.span_label(sp, "not valid as function parameter");
err.note("`self` is only valid as the first parameter of an associated function");
}
err.emit();
self.struct_span_err(sp, "unexpected `self` parameter in function")
.span_label(sp, "must be the first parameter of an associated function")
.emit();
Ok(param)
}

64 changes: 22 additions & 42 deletions src/librustc_parse/parser/item.rs
Original file line number Diff line number Diff line change
@@ -1715,8 +1715,6 @@ impl<'a> Parser<'a> {

/// The parsing configuration used to parse a parameter list (see `parse_fn_params`).
pub(super) struct ParamCfg {
/// Is `self` is allowed as the first parameter?
pub is_self_allowed: 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,
@@ -1732,8 +1730,8 @@ impl<'a> Parser<'a> {
attrs: Vec<Attribute>,
header: FnHeader,
) -> PResult<'a, Option<P<Item>>> {
let (ident, decl, generics) =
self.parse_fn_sig(ParamCfg { is_self_allowed: false, is_name_required: |_| true })?;
let cfg = ParamCfg { is_name_required: |_| true };
let (ident, decl, generics) = self.parse_fn_sig(&cfg)?;
let (inner_attrs, body) = self.parse_inner_attrs_and_block()?;
let kind = ItemKind::Fn(FnSig { decl, header }, generics, body);
self.mk_item_with_info(attrs, lo, vis, (ident, kind, Some(inner_attrs)))
@@ -1747,20 +1745,13 @@ impl<'a> Parser<'a> {
attrs: Vec<Attribute>,
extern_sp: Span,
) -> PResult<'a, P<ForeignItem>> {
let cfg = ParamCfg { is_name_required: |_| true };
self.expect_keyword(kw::Fn)?;
let (ident, decl, generics) =
self.parse_fn_sig(ParamCfg { is_self_allowed: false, is_name_required: |_| true })?;
let (ident, decl, generics) = self.parse_fn_sig(&cfg)?;
let span = lo.to(self.token.span);
self.parse_semi_or_incorrect_foreign_fn_body(&ident, extern_sp)?;
Ok(P(ast::ForeignItem {
ident,
attrs,
kind: ForeignItemKind::Fn(decl, generics),
id: DUMMY_NODE_ID,
span,
vis,
tokens: None,
}))
let kind = ForeignItemKind::Fn(decl, generics);
Ok(P(ast::ForeignItem { ident, attrs, kind, id: DUMMY_NODE_ID, span, vis, tokens: None }))
}

fn parse_assoc_fn(
@@ -1770,8 +1761,7 @@ impl<'a> Parser<'a> {
is_name_required: fn(&token::Token) -> bool,
) -> PResult<'a, (Ident, AssocItemKind, Generics)> {
let header = self.parse_fn_front_matter()?;
let (ident, decl, generics) =
self.parse_fn_sig(ParamCfg { is_self_allowed: true, is_name_required })?;
let (ident, decl, generics) = self.parse_fn_sig(&ParamCfg { is_name_required })?;
let sig = FnSig { header, decl };
let body = self.parse_assoc_fn_body(at_end, attrs)?;
Ok((ident, AssocItemKind::Fn(sig, body), generics))
@@ -1847,7 +1837,7 @@ impl<'a> Parser<'a> {
}

/// Parse the "signature", including the identifier, parameters, and generics of a function.
fn parse_fn_sig(&mut self, cfg: ParamCfg) -> PResult<'a, (Ident, P<FnDecl>, Generics)> {
fn parse_fn_sig(&mut self, cfg: &ParamCfg) -> PResult<'a, (Ident, P<FnDecl>, Generics)> {
let ident = self.parse_ident()?;
let mut generics = self.parse_generics()?;
let decl = self.parse_fn_decl(cfg, true)?;
@@ -1858,7 +1848,7 @@ impl<'a> Parser<'a> {
/// Parses the parameter list and result type of a function declaration.
pub(super) fn parse_fn_decl(
&mut self,
cfg: ParamCfg,
cfg: &ParamCfg,
ret_allow_plus: bool,
) -> PResult<'a, P<FnDecl>> {
Ok(P(FnDecl {
@@ -1868,11 +1858,11 @@ impl<'a> Parser<'a> {
}

/// Parses the parameter list of a function, including the `(` and `)` delimiters.
fn parse_fn_params(&mut self, mut cfg: ParamCfg) -> PResult<'a, Vec<Param>> {
let is_trait_item = cfg.is_self_allowed;
// Parse the arguments, starting out with `self` being possibly allowed...
fn parse_fn_params(&mut self, cfg: &ParamCfg) -> PResult<'a, Vec<Param>> {
let mut first_param = true;
// Parse the arguments, starting out with `self` being allowed...
let (mut params, _) = self.parse_paren_comma_seq(|p| {
let param = p.parse_param_general(&cfg, is_trait_item).or_else(|mut e| {
let param = p.parse_param_general(&cfg, first_param).or_else(|mut e| {
e.emit();
let lo = p.prev_span;
// Skip every token until next possible arg or end.
@@ -1881,29 +1871,25 @@ impl<'a> Parser<'a> {
Ok(dummy_arg(Ident::new(kw::Invalid, lo.to(p.prev_span))))
});
// ...now that we've parsed the first argument, `self` is no longer allowed.
cfg.is_self_allowed = false;
first_param = false;
param
})?;
// Replace duplicated recovered params with `_` pattern to avoid unnecessary errors.
self.deduplicate_recovered_params_names(&mut params);
Ok(params)
}

/// Skips unexpected attributes and doc comments in this position and emits an appropriate
/// error.
/// This version of parse param doesn't necessarily require identifier names.
fn parse_param_general(&mut self, cfg: &ParamCfg, is_trait_item: bool) -> PResult<'a, Param> {
/// Parses a single function parameter.
///
/// - `self` is syntactically allowed when `first_param` holds.
fn parse_param_general(&mut self, cfg: &ParamCfg, first_param: bool) -> PResult<'a, Param> {
let lo = self.token.span;
let attrs = self.parse_outer_attributes()?;

// Possibly parse `self`. Recover if we parsed it and it wasn't allowed here.
if let Some(mut param) = self.parse_self_param()? {
param.attrs = attrs.into();
return if cfg.is_self_allowed {
Ok(param)
} else {
self.recover_bad_self_param(param, is_trait_item)
};
return if first_param { Ok(param) } else { self.recover_bad_self_param(param) };
}

let is_name_required = match self.token.kind {
@@ -1915,13 +1901,9 @@ impl<'a> Parser<'a> {

let pat = self.parse_fn_param_pat()?;
if let Err(mut err) = self.expect(&token::Colon) {
return if let Some(ident) = self.parameter_without_type(
&mut err,
pat,
is_name_required,
cfg.is_self_allowed,
is_trait_item,
) {
return if let Some(ident) =
self.parameter_without_type(&mut err, pat, is_name_required, first_param)
{
err.emit();
Ok(dummy_arg(ident))
} else {
@@ -1975,8 +1957,6 @@ impl<'a> Parser<'a> {
}

/// Returns the parsed optional self parameter and whether a self shortcut was used.
///
/// See `parse_self_param_with_attrs` to collect attributes.
fn parse_self_param(&mut self) -> PResult<'a, Option<Param>> {
// Extract an identifier *after* having confirmed that the token is one.
let expect_self_ident = |this: &mut Self| {
3 changes: 1 addition & 2 deletions src/librustc_parse/parser/ty.rs
Original file line number Diff line number Diff line change
@@ -288,8 +288,7 @@ impl<'a> Parser<'a> {
let unsafety = self.parse_unsafety();
let ext = self.parse_extern()?;
self.expect_keyword(kw::Fn)?;
let cfg = ParamCfg { is_self_allowed: false, is_name_required: |_| false };
let decl = self.parse_fn_decl(cfg, false)?;
let decl = self.parse_fn_decl(&ParamCfg { is_name_required: |_| false }, false)?;
Ok(TyKind::BareFn(P(BareFnTy { ext, unsafety, generic_params, decl })))
}

Loading