From 1c235de97dc6105a06b525bde126ab2704d41093 Mon Sep 17 00:00:00 2001 From: Eli Friedman Date: Tue, 27 Oct 2015 23:41:32 -0700 Subject: [PATCH] Fix crash speculatively parsing macro arguments as expressions. The problem is essentially that if we try to parse a token tree using a CodeMap different from the one the tree was originally parsed with, spans become nonsense. Since CodeMaps can't be cloned, we're basically forced to use the original ParseSess for additional parsing. Ideally, rustfmt would be a bit more clever and figure out how to parse macro arguments based on the definition of the macro itself, rather than just guessing that a particular token sequence looks like an expression, but this is good enough for now. Fixes #538. --- src/expr.rs | 6 ++++-- src/lib.rs | 22 ++++++++++++++++------ src/macros.rs | 5 ++--- src/rewrite.rs | 3 +++ src/visitor.rs | 17 +++++++++-------- tests/source/macros.rs | 2 ++ tests/target/macros.rs | 2 ++ 7 files changed, 38 insertions(+), 19 deletions(-) diff --git a/src/expr.rs b/src/expr.rs index 16c46f6bef1..951b407ef43 100644 --- a/src/expr.rs +++ b/src/expr.rs @@ -422,7 +422,7 @@ fn rewrite(&self, context: &RewriteContext, width: usize, offset: Indent) -> Opt return Some(user_str); } - let mut visitor = FmtVisitor::from_codemap(context.codemap, context.config, None); + let mut visitor = FmtVisitor::from_codemap(context.parse_session, context.config, None); visitor.block_indent = context.block_indent; let prefix = match self.rules { @@ -833,7 +833,9 @@ fn rewrite(&self, context: &RewriteContext, width: usize, offset: Indent) -> Opt let attr_str = if !attrs.is_empty() { // We only use this visitor for the attributes, should we use it for // more? - let mut attr_visitor = FmtVisitor::from_codemap(context.codemap, context.config, None); + let mut attr_visitor = FmtVisitor::from_codemap(context.parse_session, + context.config, + None); attr_visitor.block_indent = context.block_indent; attr_visitor.last_pos = attrs[0].span.lo; if attr_visitor.visit_attrs(attrs) { diff --git a/src/lib.rs b/src/lib.rs index a22e797fb68..c7c2178e098 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -26,7 +26,8 @@ extern crate term; use syntax::ast; -use syntax::codemap::{CodeMap, Span}; +use syntax::codemap::Span; +use syntax::diagnostic::{EmitterWriter, Handler}; use syntax::parse::{self, ParseSess}; use std::ops::{Add, Sub}; @@ -288,11 +289,15 @@ fn fmt(&self, fmt: &mut fmt::Formatter) -> Result<(), fmt::Error> { } // Formatting which depends on the AST. -fn fmt_ast(krate: &ast::Crate, codemap: &CodeMap, config: &Config, mode: WriteMode) -> FileMap { +fn fmt_ast(krate: &ast::Crate, + parse_session: &ParseSess, + config: &Config, + mode: WriteMode) + -> FileMap { let mut file_map = FileMap::new(); - for (path, module) in modules::list_files(krate, codemap) { + for (path, module) in modules::list_files(krate, parse_session.codemap()) { let path = path.to_str().unwrap(); - let mut visitor = FmtVisitor::from_codemap(codemap, config, Some(mode)); + let mut visitor = FmtVisitor::from_codemap(parse_session, config, Some(mode)); visitor.format_separate_mod(module, path); file_map.insert(path.to_owned(), visitor.buffer); } @@ -382,9 +387,14 @@ pub fn fmt_lines(file_map: &mut FileMap, config: &Config) -> FormatReport { } pub fn format(file: &Path, config: &Config, mode: WriteMode) -> FileMap { - let parse_session = ParseSess::new(); + let mut parse_session = ParseSess::new(); let krate = parse::parse_crate_from_file(file, Vec::new(), &parse_session); - let mut file_map = fmt_ast(&krate, parse_session.codemap(), config, mode); + + // Suppress error output after parsing. + let emitter = Box::new(EmitterWriter::new(Box::new(Vec::new()), None)); + parse_session.span_diagnostic.handler = Handler::with_emitter(false, emitter); + + let mut file_map = fmt_ast(&krate, &parse_session, config, mode); // For some reason, the codemap does not include terminating // newlines so we must add one on for each file. This is sad. diff --git a/src/macros.rs b/src/macros.rs index bede00d6e9e..389e29014f0 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -21,7 +21,7 @@ use syntax::ast; use syntax::parse::token::{Eof, Comma, Token}; -use syntax::parse::{ParseSess, tts_to_parser}; +use syntax::parse::tts_to_parser; use syntax::codemap::{mk_sp, BytePos}; use Indent; @@ -73,8 +73,7 @@ pub fn rewrite_macro(mac: &ast::Mac, }; } - let parse_session = ParseSess::new(); - let mut parser = tts_to_parser(&parse_session, mac.node.tts.clone(), Vec::new()); + let mut parser = tts_to_parser(context.parse_session, mac.node.tts.clone(), Vec::new()); let mut expr_vec = Vec::new(); loop { diff --git a/src/rewrite.rs b/src/rewrite.rs index cf8be8004e1..5ca8d2e8241 100644 --- a/src/rewrite.rs +++ b/src/rewrite.rs @@ -11,6 +11,7 @@ // A generic trait to abstract the rewriting of an element (of the AST). use syntax::codemap::{CodeMap, Span}; +use syntax::parse::ParseSess; use Indent; use config::Config; @@ -27,6 +28,7 @@ pub trait Rewrite { } pub struct RewriteContext<'a> { + pub parse_session: &'a ParseSess, pub codemap: &'a CodeMap, pub config: &'a Config, // Indentation due to nesting of blocks. @@ -36,6 +38,7 @@ pub struct RewriteContext<'a> { impl<'a> RewriteContext<'a> { pub fn nested_context(&self) -> RewriteContext<'a> { RewriteContext { + parse_session: self.parse_session, codemap: self.codemap, config: self.config, block_indent: self.block_indent.block_indent(self.config), diff --git a/src/visitor.rs b/src/visitor.rs index bd30e80e5a9..534b2c877fd 100644 --- a/src/visitor.rs +++ b/src/visitor.rs @@ -10,6 +10,7 @@ use syntax::ast; use syntax::codemap::{self, CodeMap, Span, BytePos}; +use syntax::parse::ParseSess; use syntax::visit; use strings::string_buffer::StringBuffer; @@ -23,6 +24,7 @@ use items::rewrite_static; pub struct FmtVisitor<'a> { + pub parse_session: &'a ParseSess, pub codemap: &'a CodeMap, pub buffer: StringBuffer, pub last_pos: BytePos, @@ -363,12 +365,13 @@ fn push_rewrite(&mut self, span: Span, rewrite: Option) { } } - pub fn from_codemap(codemap: &'a CodeMap, + pub fn from_codemap(parse_session: &'a ParseSess, config: &'a Config, mode: Option) -> FmtVisitor<'a> { FmtVisitor { - codemap: codemap, + parse_session: parse_session, + codemap: parse_session.codemap(), buffer: StringBuffer::new(), last_pos: BytePos(0), block_indent: Indent { @@ -461,13 +464,10 @@ fn format_import(&mut self, vis: ast::Visibility, vp: &ast::ViewPath, span: Span let vis = utils::format_visibility(vis); let mut offset = self.block_indent; offset.alignment += vis.len() + "use ".len(); - let context = RewriteContext { - codemap: self.codemap, - config: self.config, - block_indent: self.block_indent, - }; // 1 = ";" - match vp.rewrite(&context, self.config.max_width - offset.width() - 1, offset) { + match vp.rewrite(&self.get_context(), + self.config.max_width - offset.width() - 1, + offset) { Some(ref s) if s.is_empty() => { // Format up to last newline let prev_span = codemap::mk_sp(self.last_pos, span.lo); @@ -493,6 +493,7 @@ fn format_import(&mut self, vis: ast::Visibility, vp: &ast::ViewPath, span: Span pub fn get_context(&self) -> RewriteContext { RewriteContext { + parse_session: self.parse_session, codemap: self.codemap, config: self.config, block_indent: self.block_indent, diff --git a/tests/source/macros.rs b/tests/source/macros.rs index 8487e0c5deb..5b07596fdf9 100644 --- a/tests/source/macros.rs +++ b/tests/source/macros.rs @@ -27,4 +27,6 @@ fn main() { hamkaas!{ () }; macrowithbraces! {dont, format, me} + + x!(fn); } diff --git a/tests/target/macros.rs b/tests/target/macros.rs index ac87b693246..2bcc6f9a339 100644 --- a/tests/target/macros.rs +++ b/tests/target/macros.rs @@ -30,4 +30,6 @@ fn main() { hamkaas!{ () }; macrowithbraces! {dont, format, me} + + x!(fn); }