From dc13072b7b7143fe98926632b9b89e7ffff2cdb7 Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Mon, 3 Jun 2019 20:47:42 -0600 Subject: [PATCH] Use Symbol for named arguments in fmt_macros --- Cargo.lock | 3 ++ src/libfmt_macros/Cargo.toml | 3 ++ src/libfmt_macros/lib.rs | 37 +++++++++++++++---------- src/librustc/traits/error_reporting.rs | 32 ++++++++++----------- src/librustc/traits/on_unimplemented.rs | 34 +++++++++++------------ src/libsyntax_ext/format.rs | 25 ++++++++--------- src/libsyntax_pos/symbol.rs | 6 ++++ 7 files changed, 79 insertions(+), 61 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7f35b7344dfed..73605af7936f2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -910,6 +910,9 @@ dependencies = [ [[package]] name = "fmt_macros" version = "0.0.0" +dependencies = [ + "syntax_pos 0.0.0", +] [[package]] name = "fnv" diff --git a/src/libfmt_macros/Cargo.toml b/src/libfmt_macros/Cargo.toml index 50779a2d9ad08..fc32f21ec4e0a 100644 --- a/src/libfmt_macros/Cargo.toml +++ b/src/libfmt_macros/Cargo.toml @@ -8,3 +8,6 @@ edition = "2018" name = "fmt_macros" path = "lib.rs" crate-type = ["dylib"] + +[dependencies] +syntax_pos = { path = "../libsyntax_pos" } diff --git a/src/libfmt_macros/lib.rs b/src/libfmt_macros/lib.rs index d5ce389ed5f7f..9d6720810b818 100644 --- a/src/libfmt_macros/lib.rs +++ b/src/libfmt_macros/lib.rs @@ -24,6 +24,8 @@ use std::str; use std::string; use std::iter; +use syntax_pos::Symbol; + /// A piece is a portion of the format string which represents the next part /// to emit. These are emitted as a stream by the `Parser` class. #[derive(Copy, Clone, PartialEq)] @@ -39,7 +41,7 @@ pub enum Piece<'a> { #[derive(Copy, Clone, PartialEq)] pub struct Argument<'a> { /// Where to find this argument - pub position: Position<'a>, + pub position: Position, /// How to format the argument pub format: FormatSpec<'a>, } @@ -54,9 +56,9 @@ pub struct FormatSpec<'a> { /// Packed version of various flags provided pub flags: u32, /// The integer precision to use - pub precision: Count<'a>, + pub precision: Count, /// The string width requested for the resulting format - pub width: Count<'a>, + pub width: Count, /// The descriptor string representing the name of the format desired for /// this argument, this can be empty or any number of characters, although /// it is required to be one word. @@ -65,16 +67,16 @@ pub struct FormatSpec<'a> { /// Enum describing where an argument for a format can be located. #[derive(Copy, Clone, PartialEq)] -pub enum Position<'a> { +pub enum Position { /// The argument is implied to be located at an index ArgumentImplicitlyIs(usize), /// The argument is located at a specific index given in the format ArgumentIs(usize), /// The argument has a name. - ArgumentNamed(&'a str), + ArgumentNamed(Symbol), } -impl Position<'_> { +impl Position { pub fn index(&self) -> Option { match self { ArgumentIs(i) | ArgumentImplicitlyIs(i) => Some(*i), @@ -119,11 +121,11 @@ pub enum Flag { /// A count is used for the precision and width parameters of an integer, and /// can reference either an argument or a literal integer. #[derive(Copy, Clone, PartialEq)] -pub enum Count<'a> { +pub enum Count { /// The count is specified explicitly. CountIs(usize), /// The count is specified by the argument with the given name. - CountIsName(&'a str), + CountIsName(Symbol), /// The count is specified by the argument at the given index. CountIsParam(usize), /// The count is implied and cannot be explicitly specified. @@ -431,12 +433,14 @@ impl<'a> Parser<'a> { /// integer index of an argument, a named argument, or a blank string. /// Returns `Some(parsed_position)` if the position is not implicitly /// consuming a macro argument, `None` if it's the case. - fn position(&mut self) -> Option> { + fn position(&mut self) -> Option { if let Some(i) = self.integer() { Some(ArgumentIs(i)) } else { match self.cur.peek() { - Some(&(_, c)) if c.is_alphabetic() => Some(ArgumentNamed(self.word())), + Some(&(_, c)) if c.is_alphabetic() => { + Some(ArgumentNamed(Symbol::intern(self.word()))) + } Some(&(pos, c)) if c == '_' => { let invalid_name = self.string(pos); self.err_with_note(format!("invalid argument name `{}`", invalid_name), @@ -444,7 +448,7 @@ impl<'a> Parser<'a> { "argument names cannot start with an underscore", self.to_span_index(pos), self.to_span_index(pos + invalid_name.len())); - Some(ArgumentNamed(invalid_name)) + Some(ArgumentNamed(Symbol::intern(invalid_name))) }, // This is an `ArgumentNext`. @@ -552,7 +556,7 @@ impl<'a> Parser<'a> { /// Parses a Count parameter at the current position. This does not check /// for 'CountIsNextParam' because that is only used in precision, not /// width. - fn count(&mut self) -> Count<'a> { + fn count(&mut self) -> Count { if let Some(i) = self.integer() { if self.consume('$') { CountIsParam(i) @@ -566,7 +570,7 @@ impl<'a> Parser<'a> { self.cur = tmp; CountImplied } else if self.consume('$') { - CountIsName(word) + CountIsName(Symbol::intern(word)) } else { self.cur = tmp; CountImplied @@ -756,6 +760,8 @@ mod tests { } #[test] fn format_counts() { + use syntax_pos::{GLOBALS, Globals, edition}; + GLOBALS.set(&Globals::new(edition::DEFAULT_EDITION), || { same("{:10s}", &[NextArgument(Argument { position: ArgumentImplicitlyIs(0), @@ -811,11 +817,12 @@ mod tests { fill: None, align: AlignUnknown, flags: 0, - precision: CountIsName("b"), - width: CountIsName("a"), + precision: CountIsName(Symbol::intern("b")), + width: CountIsName(Symbol::intern("a")), ty: "s", }, })]); + }); } #[test] fn format_flags() { diff --git a/src/librustc/traits/error_reporting.rs b/src/librustc/traits/error_reporting.rs index 490501bde73e5..50d2eeef421c1 100644 --- a/src/librustc/traits/error_reporting.rs +++ b/src/librustc/traits/error_reporting.rs @@ -353,7 +353,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { _ => { // this is a "direct", user-specified, rather than derived, // obligation. - flags.push(("direct".to_owned(), None)); + flags.push((sym::direct, None)); } } @@ -365,27 +365,27 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { // Currently I'm leaving it for what I need for `try`. if self.tcx.trait_of_item(item) == Some(trait_ref.def_id) { let method = self.tcx.item_name(item); - flags.push(("from_method".to_owned(), None)); - flags.push(("from_method".to_owned(), Some(method.to_string()))); + flags.push((sym::from_method, None)); + flags.push((sym::from_method, Some(method.to_string()))); } } if let Some(t) = self.get_parent_trait_ref(&obligation.cause.code) { - flags.push(("parent_trait".to_owned(), Some(t))); + flags.push((sym::parent_trait, Some(t))); } if let Some(k) = obligation.cause.span.compiler_desugaring_kind() { - flags.push(("from_desugaring".to_owned(), None)); - flags.push(("from_desugaring".to_owned(), Some(k.name().to_string()))); + flags.push((sym::from_desugaring, None)); + flags.push((sym::from_desugaring, Some(k.name().to_string()))); } let generics = self.tcx.generics_of(def_id); let self_ty = trait_ref.self_ty(); // This is also included through the generics list as `Self`, // but the parser won't allow you to use it - flags.push(("_Self".to_owned(), Some(self_ty.to_string()))); + flags.push((sym::_Self, Some(self_ty.to_string()))); if let Some(def) = self_ty.ty_adt_def() { // We also want to be able to select self's original // signature with no type arguments resolved - flags.push(("_Self".to_owned(), Some(self.tcx.type_of(def.did).to_string()))); + flags.push((sym::_Self, Some(self.tcx.type_of(def.did).to_string()))); } for param in generics.params.iter() { @@ -396,38 +396,38 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { }, GenericParamDefKind::Lifetime => continue, }; - let name = param.name.to_string(); + let name = param.name.as_symbol(); flags.push((name, Some(value))); } if let Some(true) = self_ty.ty_adt_def().map(|def| def.did.is_local()) { - flags.push(("crate_local".to_owned(), None)); + flags.push((sym::crate_local, None)); } // Allow targeting all integers using `{integral}`, even if the exact type was resolved if self_ty.is_integral() { - flags.push(("_Self".to_owned(), Some("{integral}".to_owned()))); + flags.push((sym::_Self, Some("{integral}".to_owned()))); } if let ty::Array(aty, len) = self_ty.sty { - flags.push(("_Self".to_owned(), Some("[]".to_owned()))); - flags.push(("_Self".to_owned(), Some(format!("[{}]", aty)))); + flags.push((sym::_Self, Some("[]".to_owned()))); + flags.push((sym::_Self, Some(format!("[{}]", aty)))); if let Some(def) = aty.ty_adt_def() { // We also want to be able to select the array's type's original // signature with no type arguments resolved flags.push(( - "_Self".to_owned(), + sym::_Self, Some(format!("[{}]", self.tcx.type_of(def.did).to_string())), )); let tcx = self.tcx; if let Some(len) = len.assert_usize(tcx) { flags.push(( - "_Self".to_owned(), + sym::_Self, Some(format!("[{}; {}]", self.tcx.type_of(def.did).to_string(), len)), )); } else { flags.push(( - "_Self".to_owned(), + sym::_Self, Some(format!("[{}; _]", self.tcx.type_of(def.did).to_string())), )); } diff --git a/src/librustc/traits/on_unimplemented.rs b/src/librustc/traits/on_unimplemented.rs index 14b968c83b2ea..b78396c90dc65 100644 --- a/src/librustc/traits/on_unimplemented.rs +++ b/src/librustc/traits/on_unimplemented.rs @@ -7,7 +7,7 @@ use crate::util::nodemap::FxHashMap; use syntax::ast::{MetaItem, NestedMetaItem}; use syntax::attr; -use syntax::symbol::sym; +use syntax::symbol::{Symbol, kw, sym}; use syntax_pos::Span; use syntax_pos::symbol::LocalInternedString; @@ -167,7 +167,7 @@ impl<'a, 'gcx, 'tcx> OnUnimplementedDirective { pub fn evaluate(&self, tcx: TyCtxt<'a, 'gcx, 'tcx>, trait_ref: ty::TraitRef<'tcx>, - options: &[(String, Option)]) + options: &[(Symbol, Option)]) -> OnUnimplementedNote { let mut message = None; @@ -180,7 +180,7 @@ impl<'a, 'gcx, 'tcx> OnUnimplementedDirective { if !attr::eval_condition(condition, &tcx.sess.parse_sess, &mut |c| { c.ident().map_or(false, |ident| { options.contains(&( - ident.to_string(), + ident.name, c.value_str().map(|s| s.as_str().to_string()) )) }) @@ -203,8 +203,8 @@ impl<'a, 'gcx, 'tcx> OnUnimplementedDirective { } } - let options: FxHashMap = options.into_iter() - .filter_map(|(k, v)| v.as_ref().map(|v| (k.to_owned(), v.to_owned()))) + let options: FxHashMap = options.into_iter() + .filter_map(|(k, v)| v.as_ref().map(|v| (*k, v.to_owned()))) .collect(); OnUnimplementedNote { label: label.map(|l| l.format(tcx, trait_ref, &options)), @@ -241,16 +241,16 @@ impl<'a, 'gcx, 'tcx> OnUnimplementedFormatString { Piece::String(_) => (), // Normal string, no need to check it Piece::NextArgument(a) => match a.position { // `{Self}` is allowed - Position::ArgumentNamed(s) if s == "Self" => (), + Position::ArgumentNamed(s) if s == kw::SelfUpper => (), // `{ThisTraitsName}` is allowed - Position::ArgumentNamed(s) if s == name.as_str() => (), + Position::ArgumentNamed(s) if s == name => (), // `{from_method}` is allowed - Position::ArgumentNamed(s) if s == "from_method" => (), + Position::ArgumentNamed(s) if s == sym::from_method => (), // `{from_desugaring}` is allowed - Position::ArgumentNamed(s) if s == "from_desugaring" => (), + Position::ArgumentNamed(s) if s == sym::from_desugaring => (), // So is `{A}` if A is a type parameter Position::ArgumentNamed(s) => match generics.params.iter().find(|param| { - param.name.as_str() == s + param.name.as_symbol() == s }) { Some(_) => (), None => { @@ -276,7 +276,7 @@ impl<'a, 'gcx, 'tcx> OnUnimplementedFormatString { &self, tcx: TyCtxt<'a, 'gcx, 'tcx>, trait_ref: ty::TraitRef<'tcx>, - options: &FxHashMap, + options: &FxHashMap, ) -> String { let name = tcx.item_name(trait_ref.def_id); let trait_str = tcx.def_path_str(trait_ref.def_id); @@ -289,9 +289,9 @@ impl<'a, 'gcx, 'tcx> OnUnimplementedFormatString { }, GenericParamDefKind::Lifetime => return None }; - let name = param.name.to_string(); + let name = param.name.as_symbol(); Some((name, value)) - }).collect::>(); + }).collect::>(); let empty_string = String::new(); let parser = Parser::new(&self.0, None, vec![], false); @@ -299,15 +299,15 @@ impl<'a, 'gcx, 'tcx> OnUnimplementedFormatString { match p { Piece::String(s) => s, Piece::NextArgument(a) => match a.position { - Position::ArgumentNamed(s) => match generic_map.get(s) { + Position::ArgumentNamed(s) => match generic_map.get(&s) { Some(val) => val, - None if s == name.as_str() => { + None if s == name => { &trait_str } None => { - if let Some(val) = options.get(s) { + if let Some(val) = options.get(&s) { val - } else if s == "from_desugaring" || s == "from_method" { + } else if s == sym::from_desugaring || s == sym::from_method { // don't break messages using these two arguments incorrectly &empty_string } else { diff --git a/src/libsyntax_ext/format.rs b/src/libsyntax_ext/format.rs index 377164728f42a..2863daa13a99c 100644 --- a/src/libsyntax_ext/format.rs +++ b/src/libsyntax_ext/format.rs @@ -28,7 +28,7 @@ enum ArgumentType { enum Position { Exact(usize), - Named(String), + Named(Symbol), } struct Context<'a, 'b: 'a> { @@ -57,7 +57,7 @@ struct Context<'a, 'b: 'a> { /// Unique format specs seen for each argument. arg_unique_types: Vec>, /// Map from named arguments to their resolved indices. - names: FxHashMap, + names: FxHashMap, /// The latest consecutive literal strings, or empty if there weren't any. literal: String, @@ -127,9 +127,9 @@ fn parse_args<'a>( ecx: &mut ExtCtxt<'a>, sp: Span, tts: &[tokenstream::TokenTree] -) -> Result<(P, Vec>, FxHashMap), DiagnosticBuilder<'a>> { +) -> Result<(P, Vec>, FxHashMap), DiagnosticBuilder<'a>> { let mut args = Vec::>::new(); - let mut names = FxHashMap::::default(); + let mut names = FxHashMap::::default(); let mut p = ecx.new_parser_from_tts(tts); @@ -158,11 +158,10 @@ fn parse_args<'a>( "expected ident, positional arguments cannot follow named arguments", )); }; - let name: &str = &name.as_str(); p.expect(&token::Eq)?; let e = p.parse_expr()?; - if let Some(prev) = names.get(name) { + if let Some(prev) = names.get(&name) { ecx.struct_span_err(e.span, &format!("duplicate argument named `{}`", name)) .span_note(args[*prev].span, "previously here") .emit(); @@ -174,7 +173,7 @@ fn parse_args<'a>( // if the input is valid, we can simply append to the positional // args. And remember the names. let slot = args.len(); - names.insert(name.to_string(), slot); + names.insert(name, slot); args.push(e); } else { let e = p.parse_expr()?; @@ -188,7 +187,7 @@ impl<'a, 'b> Context<'a, 'b> { fn resolve_name_inplace(&self, p: &mut parse::Piece<'_>) { // NOTE: the `unwrap_or` branch is needed in case of invalid format // arguments, e.g., `format_args!("{foo}")`. - let lookup = |s| *self.names.get(s).unwrap_or(&0); + let lookup = |s: Symbol| *self.names.get(&s).unwrap_or(&0); match *p { parse::String(_) => {} @@ -222,7 +221,7 @@ impl<'a, 'b> Context<'a, 'b> { // it's written second, so it should come after width/precision. let pos = match arg.position { parse::ArgumentIs(i) | parse::ArgumentImplicitlyIs(i) => Exact(i), - parse::ArgumentNamed(s) => Named(s.to_string()), + parse::ArgumentNamed(s) => Named(s), }; let ty = Placeholder(arg.format.ty.to_string()); @@ -232,7 +231,7 @@ impl<'a, 'b> Context<'a, 'b> { } } - fn verify_count(&mut self, c: parse::Count<'_>) { + fn verify_count(&mut self, c: parse::Count) { match c { parse::CountImplied | parse::CountIs(..) => {} @@ -240,7 +239,7 @@ impl<'a, 'b> Context<'a, 'b> { self.verify_arg_type(Exact(i), Count); } parse::CountIsName(s) => { - self.verify_arg_type(Named(s.to_string()), Count); + self.verify_arg_type(Named(s), Count); } } } @@ -390,7 +389,7 @@ impl<'a, 'b> Context<'a, 'b> { ecx.std_path(&[sym::fmt, sym::rt, sym::v1, Symbol::intern(s)]) } - fn build_count(&self, c: parse::Count<'_>) -> P { + fn build_count(&self, c: parse::Count) -> P { let sp = self.macsp; let count = |c, arg| { let mut path = Context::rtpath(self.ecx, "Count"); @@ -739,7 +738,7 @@ pub fn expand_preparsed_format_args(ecx: &mut ExtCtxt<'_>, sp: Span, efmt: P, args: Vec>, - names: FxHashMap, + names: FxHashMap, append_newline: bool) -> P { // NOTE: this verbose way of initializing `Vec>` is because diff --git a/src/libsyntax_pos/symbol.rs b/src/libsyntax_pos/symbol.rs index 302b3c75263cf..95d74ce054ef9 100644 --- a/src/libsyntax_pos/symbol.rs +++ b/src/libsyntax_pos/symbol.rs @@ -203,6 +203,7 @@ symbols! { core_intrinsics, crate_id, crate_in_paths, + crate_local, crate_name, crate_type, crate_visibility_modifier, @@ -221,6 +222,7 @@ symbols! { deref, deref_mut, derive, + direct, doc, doc_alias, doc_cfg, @@ -278,8 +280,10 @@ symbols! { format_args_nl, from, From, + from_desugaring, from_error, from_generator, + from_method, from_ok, from_usize, fundamental, @@ -443,6 +447,7 @@ symbols! { panic_impl, panic_implementation, panic_runtime, + parent_trait, partial_cmp, PartialOrd, passes, @@ -569,6 +574,7 @@ symbols! { __rust_unstable_column, rvalue_static_promotion, sanitizer_runtime, + _Self, self_in_typedefs, self_struct_ctor, Send,