From 37a3b7c80e3ceb31aa73640506501cca4b973647 Mon Sep 17 00:00:00 2001 From: ljedrz Date: Fri, 14 Dec 2018 10:43:10 +0100 Subject: [PATCH 1/5] format: remove unreachable condition --- src/libsyntax_ext/format.rs | 31 +++++++++---------------------- 1 file changed, 9 insertions(+), 22 deletions(-) diff --git a/src/libsyntax_ext/format.rs b/src/libsyntax_ext/format.rs index 24108a30fdce2..dff5309dd950f 100644 --- a/src/libsyntax_ext/format.rs +++ b/src/libsyntax_ext/format.rs @@ -158,28 +158,15 @@ fn parse_args(ecx: &mut ExtCtxt, } // accept trailing commas if named || (p.token.is_ident() && p.look_ahead(1, |t| *t == token::Eq)) { named = true; - let ident = match p.token { - token::Ident(i, _) => { - p.bump(); - i - } - _ if named => { - ecx.span_err( - p.span, - "expected ident, positional arguments cannot follow named arguments", - ); - return None; - } - _ => { - ecx.span_err( - p.span, - &format!( - "expected ident for named argument, found `{}`", - p.this_token_to_string() - ), - ); - return None; - } + let ident = if let token::Ident(i, _) = p.token { + p.bump(); + i + } else { + ecx.span_err( + p.span, + "expected ident, positional arguments cannot follow named arguments", + ); + return None; }; let name: &str = &ident.as_str(); From 3d052c920bc6ee6f5f0575f76980f0f69327662f Mon Sep 17 00:00:00 2001 From: ljedrz Date: Fri, 14 Dec 2018 10:48:41 +0100 Subject: [PATCH 2/5] format: inline one-liners related to parse_expr --- src/libsyntax/parse/parser.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index ded6da9f3adb8..071d18945c5d8 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -3051,6 +3051,7 @@ impl<'a> Parser<'a> { /// /// This parses an expression accounting for associativity and precedence of the operators in /// the expression. + #[inline] fn parse_assoc_expr(&mut self, already_parsed_attrs: Option>) -> PResult<'a, P> { @@ -3711,6 +3712,7 @@ impl<'a> Parser<'a> { } /// Parse an expression + #[inline] pub fn parse_expr(&mut self) -> PResult<'a, P> { self.parse_expr_res(Restrictions::empty(), None) } @@ -3730,6 +3732,7 @@ impl<'a> Parser<'a> { } /// Parse an expression, subject to the given restrictions + #[inline] fn parse_expr_res(&mut self, r: Restrictions, already_parsed_attrs: Option>) -> PResult<'a, P> { From 8866f68a4d038fbb26799f792dcb60d1c5e184cd Mon Sep 17 00:00:00 2001 From: ljedrz Date: Fri, 14 Dec 2018 11:28:09 +0100 Subject: [PATCH 3/5] format: refactor report_invalid_references --- src/libsyntax_ext/format.rs | 46 +++++++++++++++++-------------------- 1 file changed, 21 insertions(+), 25 deletions(-) diff --git a/src/libsyntax_ext/format.rs b/src/libsyntax_ext/format.rs index dff5309dd950f..f2122a57d2724 100644 --- a/src/libsyntax_ext/format.rs +++ b/src/libsyntax_ext/format.rs @@ -273,11 +273,11 @@ impl<'a, 'b> Context<'a, 'b> { } else { MultiSpan::from_span(self.fmtsp) }; - let mut refs: Vec<_> = self + let refs_len = self.invalid_refs.len(); + let mut refs = self .invalid_refs .iter() - .map(|(r, pos)| (r.to_string(), self.arg_spans.get(*pos))) - .collect(); + .map(|(r, pos)| (r.to_string(), self.arg_spans.get(*pos))); if self.names.is_empty() && !numbered_position_args { e = self.ecx.mut_span_err( @@ -290,28 +290,24 @@ impl<'a, 'b> Context<'a, 'b> { ), ); } else { - let (arg_list, mut sp) = match refs.len() { - 1 => { - let (reg, pos) = refs.pop().unwrap(); - ( - format!("argument {}", reg), - MultiSpan::from_span(*pos.unwrap_or(&self.fmtsp)), - ) - } - _ => { - let pos = - MultiSpan::from_spans(refs.iter().map(|(_, p)| *p.unwrap()).collect()); - let mut refs: Vec = refs.iter().map(|(s, _)| s.to_owned()).collect(); - let reg = refs.pop().unwrap(); - ( - format!( - "arguments {head} and {tail}", - tail = reg, - head = refs.join(", ") - ), - pos, - ) - } + let (arg_list, mut sp) = if refs_len == 1 { + let (reg, pos) = refs.next().unwrap(); + ( + format!("argument {}", reg), + MultiSpan::from_span(*pos.unwrap_or(&self.fmtsp)), + ) + } else { + let (mut refs, spans): (Vec<_>, Vec<_>) = refs.unzip(); + let pos = MultiSpan::from_spans(spans.into_iter().map(|s| *s.unwrap()).collect()); + let reg = refs.pop().unwrap(); + ( + format!( + "arguments {head} and {tail}", + head = refs.join(", "), + tail = reg, + ), + pos, + ) }; if !self.is_literal { sp = MultiSpan::from_span(self.fmtsp); From 002310a496e92315b3544dbd1a40f8fff190881e Mon Sep 17 00:00:00 2001 From: ljedrz Date: Fri, 14 Dec 2018 12:39:01 +0100 Subject: [PATCH 4/5] format: refactor verify_arg_type --- src/libsyntax_ext/format.rs | 38 ++++++++++++++++--------------------- 1 file changed, 16 insertions(+), 22 deletions(-) diff --git a/src/libsyntax_ext/format.rs b/src/libsyntax_ext/format.rs index f2122a57d2724..2e5698c1a6c80 100644 --- a/src/libsyntax_ext/format.rs +++ b/src/libsyntax_ext/format.rs @@ -336,33 +336,30 @@ impl<'a, 'b> Context<'a, 'b> { Placeholder(_) => { // record every (position, type) combination only once let ref mut seen_ty = self.arg_unique_types[arg]; - let i = match seen_ty.iter().position(|x| *x == ty) { - Some(i) => i, - None => { - let i = seen_ty.len(); - seen_ty.push(ty); - i - } - }; + let i = seen_ty.iter().position(|x| *x == ty).unwrap_or_else(|| { + let i = seen_ty.len(); + seen_ty.push(ty); + i + }); self.arg_types[arg].push(i); } Count => { - match self.count_positions.entry(arg) { - Entry::Vacant(e) => { - let i = self.count_positions_count; - e.insert(i); - self.count_args.push(Exact(arg)); - self.count_positions_count += 1; - } - Entry::Occupied(_) => {} + if let Entry::Vacant(e) = self.count_positions.entry(arg) { + let i = self.count_positions_count; + e.insert(i); + self.count_args.push(Exact(arg)); + self.count_positions_count += 1; } } } } Named(name) => { - let idx = match self.names.get(&name) { - Some(e) => *e, + match self.names.get(&name) { + Some(idx) => { + // Treat as positional arg. + self.verify_arg_type(Exact(*idx), ty) + } None => { let msg = format!("there is no argument named `{}`", name); let sp = if self.is_literal { @@ -372,11 +369,8 @@ impl<'a, 'b> Context<'a, 'b> { }; let mut err = self.ecx.struct_span_err(sp, &msg[..]); err.emit(); - return; } - }; - // Treat as positional arg. - self.verify_arg_type(Exact(idx), ty) + } } } } From 959313aad0a7d2eb68bd15894b5e6f94580f4582 Mon Sep 17 00:00:00 2001 From: ljedrz Date: Fri, 14 Dec 2018 14:40:05 +0100 Subject: [PATCH 5/5] format: minor stylistic improvements --- src/libsyntax_ext/format.rs | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/src/libsyntax_ext/format.rs b/src/libsyntax_ext/format.rs index 2e5698c1a6c80..41799eede9e86 100644 --- a/src/libsyntax_ext/format.rs +++ b/src/libsyntax_ext/format.rs @@ -413,12 +413,10 @@ impl<'a, 'b> Context<'a, 'b> { parse::CountIs(i) => count("Is", Some(self.ecx.expr_usize(sp, i))), parse::CountIsParam(i) => { // This needs mapping too, as `i` is referring to a macro - // argument. - let i = match self.count_positions.get(&i) { - Some(&i) => i, - None => 0, // error already emitted elsewhere - }; - let i = i + self.count_args_index_offset; + // argument. If `i` is not found in `count_positions` then + // the error had already been emitted elsewhere. + let i = self.count_positions.get(&i).cloned().unwrap_or(0) + + self.count_args_index_offset; count("Param", Some(self.ecx.expr_usize(sp, i))) } parse::CountImplied => count("Implied", None), @@ -503,10 +501,7 @@ impl<'a, 'b> Context<'a, 'b> { }, }; - let fill = match arg.format.fill { - Some(c) => c, - None => ' ', - }; + let fill = arg.format.fill.unwrap_or(' '); if *arg != simple_arg || fill != ' ' { self.all_pieces_simple = false; @@ -805,8 +800,7 @@ pub fn expand_preparsed_format_args(ecx: &mut ExtCtxt, if !parser.errors.is_empty() { let err = parser.errors.remove(0); let sp = fmt.span.from_inner_byte_pos(err.start, err.end); - let mut e = ecx.struct_span_err(sp, &format!("invalid format string: {}", - err.description)); + let mut e = ecx.struct_span_err(sp, &format!("invalid format string: {}", err.description)); e.span_label(sp, err.label + " in format string"); if let Some(note) = err.note { e.note(¬e);