Skip to content

Commit

Permalink
Use Symbol for named arguments in fmt_macros
Browse files Browse the repository at this point in the history
  • Loading branch information
@Mark-Simulacrum
Mark-Simulacrum committed Jun 9, 2019
1 parent 7795b15 commit dc13072
Show file tree
Hide file tree
Showing 7 changed files with 79 additions and 61 deletions.
3 changes: 3 additions & 0 deletions Cargo.lock
View file
Expand Up @@ -910,6 +910,9 @@ dependencies = [
[[package]]
name = "fmt_macros"
version = "0.0.0"
dependencies = [
"syntax_pos 0.0.0",
]

[[package]]
name = "fnv"
Expand Down
3 changes: 3 additions & 0 deletions src/libfmt_macros/Cargo.toml
View file
Expand Up @@ -8,3 +8,6 @@ edition = "2018"
name = "fmt_macros"
path = "lib.rs"
crate-type = ["dylib"]

[dependencies]
syntax_pos = { path = "../libsyntax_pos" }
37 changes: 22 additions & 15 deletions src/libfmt_macros/lib.rs
View file
Expand Up @@ -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)]
Expand All @@ -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>,
}
Expand All @@ -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.
Expand All @@ -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<usize> {
match self {
ArgumentIs(i) | ArgumentImplicitlyIs(i) => Some(*i),
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -431,20 +433,22 @@ 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<Position<'a>> {
fn position(&mut self) -> Option<Position> {
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),
"invalid argument name",
"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`.
Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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() {
Expand Down
32 changes: 16 additions & 16 deletions src/librustc/traits/error_reporting.rs
View file
Expand Up @@ -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));
}
}

Expand All @@ -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() {
Expand All @@ -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())),
));
}
Expand Down
34 changes: 17 additions & 17 deletions src/librustc/traits/on_unimplemented.rs
View file
Expand Up @@ -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;

Expand Down Expand Up @@ -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<String>)])
options: &[(Symbol, Option<String>)])
-> OnUnimplementedNote
{
let mut message = None;
Expand All @@ -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())
))
})
Expand All @@ -203,8 +203,8 @@ impl<'a, 'gcx, 'tcx> OnUnimplementedDirective {
}
}

let options: FxHashMap<String, String> = options.into_iter()
.filter_map(|(k, v)| v.as_ref().map(|v| (k.to_owned(), v.to_owned())))
let options: FxHashMap<Symbol, String> = 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)),
Expand Down Expand Up @@ -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 => {
Expand All @@ -276,7 +276,7 @@ impl<'a, 'gcx, 'tcx> OnUnimplementedFormatString {
&self,
tcx: TyCtxt<'a, 'gcx, 'tcx>,
trait_ref: ty::TraitRef<'tcx>,
options: &FxHashMap<String, String>,
options: &FxHashMap<Symbol, String>,
) -> String {
let name = tcx.item_name(trait_ref.def_id);
let trait_str = tcx.def_path_str(trait_ref.def_id);
Expand All @@ -289,25 +289,25 @@ 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::<FxHashMap<String, String>>();
}).collect::<FxHashMap<Symbol, String>>();
let empty_string = String::new();

let parser = Parser::new(&self.0, None, vec![], false);
parser.map(|p|
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 {
Expand Down

0 comments on commit dc13072

Please sign in to comment.