From b79fdb8e3c75100f465e0d5b7c6926f192e7fdbd Mon Sep 17 00:00:00 2001 From: Wesley Norris Date: Fri, 5 Oct 2018 19:22:19 -0400 Subject: [PATCH 1/8] Replaces fn main search and extern crate search with proper parsing. --- src/librustdoc/test.rs | 95 +++++++++++++++++++++++++++++++++++------- 1 file changed, 81 insertions(+), 14 deletions(-) diff --git a/src/librustdoc/test.rs b/src/librustdoc/test.rs index 2e6e76b5a4039..3f031c716676d 100644 --- a/src/librustdoc/test.rs +++ b/src/librustdoc/test.rs @@ -402,10 +402,56 @@ pub fn make_test(s: &str, // are intended to be crate attributes. prog.push_str(&crate_attrs); + // Uses libsyntax to parse the doctest and find if there's a main fn and the extern + // crate already is included. + let (already_has_main, already_has_extern_crate) = crate::syntax::with_globals(|| { + use crate::syntax::{ast, parse::{self, ParseSess}, source_map::FilePathMapping}; + use crate::syntax_pos::FileName; + + let filename = FileName::Anon; + let source = s.to_owned(); + let sess = ParseSess::new(FilePathMapping::empty()); + + let mut parser = parse::new_parser_from_source_str(&sess, filename, source); + + let mut found_main = false; + let mut found_extern_crate = cratename.is_none(); + + while let Ok(Some(item)) = parser.parse_item() { + if !found_main { + if let ast::ItemKind::Fn(..) = item.node { + if item.ident.as_str() == "main" { + found_main = true; + } + } + } + + if !found_extern_crate { + if let ast::ItemKind::ExternCrate(original) = item.node { + // This code will never be reached if `cratename` is none ecause + // `found_extern_crate` is initialized to `true` if it is none. + let cratename = cratename.unwrap(); + + match original { + Some(name) => found_extern_crate = name.as_str() == cratename, + None => found_extern_crate = item.ident.as_str() == cratename, + } + } + } + + if found_main && found_extern_crate { + break; + } + } + + (found_main, found_extern_crate) + }); + // Don't inject `extern crate std` because it's already injected by the // compiler. - if !s.contains("extern crate") && !opts.no_crate_inject && cratename != Some("std") { + if !already_has_extern_crate && !opts.no_crate_inject && cratename != Some("std") { if let Some(cratename) = cratename { + // Make sure its actually used if not included. if s.contains(cratename) { prog.push_str(&format!("extern crate {};\n", cratename)); line_offset += 1; @@ -413,19 +459,6 @@ pub fn make_test(s: &str, } } - // FIXME (#21299): prefer libsyntax or some other actual parser over this - // best-effort ad hoc approach - let already_has_main = s.lines() - .map(|line| { - let comment = line.find("//"); - if let Some(comment_begins) = comment { - &line[0..comment_begins] - } else { - line - } - }) - .any(|code| code.contains("fn main")); - if dont_insert_main || already_has_main { prog.push_str(everything_else); } else { @@ -1014,4 +1047,38 @@ assert_eq!(2+2, 4); let output = make_test(input, None, false, &opts); assert_eq!(output, (expected, 1)); } + + #[test] + fn make_test_issues_21299_33731() { + let opts = TestOptions::default(); + + let input = +"// fn main +assert_eq!(2+2, 4);"; + + let expected = +"#![allow(unused)] +fn main() { +// fn main +assert_eq!(2+2, 4); +}".to_string(); + + let output = make_test(input, None, false, &opts); + assert_eq!(output, (expected, 2)); + + let input = +"extern crate hella_qwop; +assert_eq!(asdf::foo, 4);"; + + let expected = +"#![allow(unused)] +extern crate hella_qwop; +extern crate asdf; +fn main() { +assert_eq!(asdf::foo, 4); +}".to_string(); + + let output = make_test(input, Some("asdf"), false, &opts); + assert_eq!(output, (expected, 3)); + } } From 43b65b51e0f7de77f93c65a0a807ef5ce76ac02d Mon Sep 17 00:00:00 2001 From: Wesley Norris Date: Sun, 7 Oct 2018 12:36:47 -0400 Subject: [PATCH 2/8] Tidy up source file and fix typo. --- src/librustdoc/test.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/librustdoc/test.rs b/src/librustdoc/test.rs index 3f031c716676d..8a3fe7b0babbd 100644 --- a/src/librustdoc/test.rs +++ b/src/librustdoc/test.rs @@ -407,7 +407,7 @@ pub fn make_test(s: &str, let (already_has_main, already_has_extern_crate) = crate::syntax::with_globals(|| { use crate::syntax::{ast, parse::{self, ParseSess}, source_map::FilePathMapping}; use crate::syntax_pos::FileName; - + let filename = FileName::Anon; let source = s.to_owned(); let sess = ParseSess::new(FilePathMapping::empty()); @@ -424,11 +424,11 @@ pub fn make_test(s: &str, found_main = true; } } - } - - if !found_extern_crate { + } + + if !found_extern_crate { if let ast::ItemKind::ExternCrate(original) = item.node { - // This code will never be reached if `cratename` is none ecause + // This code will never be reached if `cratename` is none because // `found_extern_crate` is initialized to `true` if it is none. let cratename = cratename.unwrap(); @@ -1051,7 +1051,7 @@ assert_eq!(2+2, 4); #[test] fn make_test_issues_21299_33731() { let opts = TestOptions::default(); - + let input = "// fn main assert_eq!(2+2, 4);"; From cd407dfe74b3a4e3df1b4b747945473dbfee10dd Mon Sep 17 00:00:00 2001 From: Wesley Norris Date: Sat, 20 Oct 2018 14:45:44 -0400 Subject: [PATCH 3/8] Separates inner attributes from code during doctest parsing. --- src/librustdoc/test.rs | 60 ++++++++++++++++++++++++++---------------- 1 file changed, 37 insertions(+), 23 deletions(-) diff --git a/src/librustdoc/test.rs b/src/librustdoc/test.rs index 8a3fe7b0babbd..1a51b8b458c4b 100644 --- a/src/librustdoc/test.rs +++ b/src/librustdoc/test.rs @@ -378,7 +378,7 @@ pub fn make_test(s: &str, dont_insert_main: bool, opts: &TestOptions) -> (String, usize) { - let (crate_attrs, everything_else) = partition_source(s); + let (crate_attrs, everything_else, crates) = partition_source(s); let everything_else = everything_else.trim(); let mut line_offset = 0; let mut prog = String::new(); @@ -409,7 +409,7 @@ pub fn make_test(s: &str, use crate::syntax_pos::FileName; let filename = FileName::Anon; - let source = s.to_owned(); + let source = crates + &everything_else; let sess = ParseSess::new(FilePathMapping::empty()); let mut parser = parse::new_parser_from_source_str(&sess, filename, source); @@ -417,30 +417,39 @@ pub fn make_test(s: &str, let mut found_main = false; let mut found_extern_crate = cratename.is_none(); - while let Ok(Some(item)) = parser.parse_item() { - if !found_main { - if let ast::ItemKind::Fn(..) = item.node { - if item.ident.as_str() == "main" { - found_main = true; + loop { + match parser.parse_item() { + Ok(Some(item)) => { + if !found_main { + if let ast::ItemKind::Fn(..) = item.node { + if item.ident.as_str() == "main" { + found_main = true; + } + } } - } - } - if !found_extern_crate { - if let ast::ItemKind::ExternCrate(original) = item.node { - // This code will never be reached if `cratename` is none because - // `found_extern_crate` is initialized to `true` if it is none. - let cratename = cratename.unwrap(); + if !found_extern_crate { + if let ast::ItemKind::ExternCrate(original) = item.node { + // This code will never be reached if `cratename` is none because + // `found_extern_crate` is initialized to `true` if it is none. + let cratename = cratename.unwrap(); + + match original { + Some(name) => found_extern_crate = name.as_str() == cratename, + None => found_extern_crate = item.ident.as_str() == cratename, + } + } + } - match original { - Some(name) => found_extern_crate = name.as_str() == cratename, - None => found_extern_crate = item.ident.as_str() == cratename, + if found_main && found_extern_crate { + break; } } - } - - if found_main && found_extern_crate { - break; + Ok(None) => break, + Err(mut e) => { + e.cancel(); + break; + } } } @@ -474,9 +483,10 @@ pub fn make_test(s: &str, } // FIXME(aburka): use a real parser to deal with multiline attributes -fn partition_source(s: &str) -> (String, String) { +fn partition_source(s: &str) -> (String, String, String) { let mut after_header = false; let mut before = String::new(); + let mut crates = String::new(); let mut after = String::new(); for line in s.lines() { @@ -490,12 +500,16 @@ fn partition_source(s: &str) -> (String, String) { after.push_str(line); after.push_str("\n"); } else { + if trimline.starts_with("#[macro_use] extern crate") + || trimline.starts_with("extern crate") { + crates.push_str(line); + } before.push_str(line); before.push_str("\n"); } } - (before, after) + (before, after, crates) } pub trait Tester { From f36ed5b58d28dabad0d04903443df13a86b46aad Mon Sep 17 00:00:00 2001 From: QuietMisdreavus Date: Thu, 1 Nov 2018 08:58:04 -0500 Subject: [PATCH 4/8] add env-logger to error-index-generator --- src/tools/error_index_generator/main.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/tools/error_index_generator/main.rs b/src/tools/error_index_generator/main.rs index 40917cc5db0d5..a5556e1d570eb 100644 --- a/src/tools/error_index_generator/main.rs +++ b/src/tools/error_index_generator/main.rs @@ -10,6 +10,7 @@ #![feature(rustc_private)] +extern crate env_logger; extern crate syntax; extern crate rustdoc; extern crate serialize as rustc_serialize; @@ -264,6 +265,7 @@ fn parse_args() -> (OutputFormat, PathBuf) { } fn main() { + env_logger::init(); PLAYGROUND.with(|slot| { *slot.borrow_mut() = Some((None, String::from("https://play.rust-lang.org/"))); }); From d6d8c6bd713e9dfe21e159ef54d1d7082b50384b Mon Sep 17 00:00:00 2001 From: QuietMisdreavus Date: Thu, 1 Nov 2018 08:58:28 -0500 Subject: [PATCH 5/8] add a line between extracted crates and everything else --- src/librustdoc/test.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/librustdoc/test.rs b/src/librustdoc/test.rs index 1a51b8b458c4b..1aca35be7ce53 100644 --- a/src/librustdoc/test.rs +++ b/src/librustdoc/test.rs @@ -412,6 +412,8 @@ pub fn make_test(s: &str, let source = crates + &everything_else; let sess = ParseSess::new(FilePathMapping::empty()); + debug!("about to parse: \n{}", source); + let mut parser = parse::new_parser_from_source_str(&sess, filename, source); let mut found_main = false; @@ -503,6 +505,7 @@ fn partition_source(s: &str) -> (String, String, String) { if trimline.starts_with("#[macro_use] extern crate") || trimline.starts_with("extern crate") { crates.push_str(line); + crates.push_str("\n"); } before.push_str(line); before.push_str("\n"); From 8a3b5e99addbda7e8c15aa1641f4d8ea517e1be4 Mon Sep 17 00:00:00 2001 From: QuietMisdreavus Date: Thu, 1 Nov 2018 09:20:08 -0500 Subject: [PATCH 6/8] silence errors found during doctest pre-parsing --- src/librustdoc/test.rs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/librustdoc/test.rs b/src/librustdoc/test.rs index 1aca35be7ce53..442e9bf6b9363 100644 --- a/src/librustdoc/test.rs +++ b/src/librustdoc/test.rs @@ -407,13 +407,23 @@ pub fn make_test(s: &str, let (already_has_main, already_has_extern_crate) = crate::syntax::with_globals(|| { use crate::syntax::{ast, parse::{self, ParseSess}, source_map::FilePathMapping}; use crate::syntax_pos::FileName; + use errors::emitter::EmitterWriter; + use errors::Handler; let filename = FileName::Anon; let source = crates + &everything_else; - let sess = ParseSess::new(FilePathMapping::empty()); + + // any errors in parsing should also appear when the doctest is compiled for real, so just + // send all the errors that libsyntax emits directly into a Sink instead of stderr + let cm = Lrc::new(SourceMap::new(FilePathMapping::empty())); + let emitter = EmitterWriter::new(box io::sink(), None, false, false); + let handler = Handler::with_emitter(false, false, box emitter); + let sess = ParseSess::with_span_handler(handler, cm); debug!("about to parse: \n{}", source); + // FIXME(misdreavus): this can still emit a FatalError (and thus halt rustdoc prematurely) + // if there is a lexing error in the first token let mut parser = parse::new_parser_from_source_str(&sess, filename, source); let mut found_main = false; From 0fe6aae49a1482c5cc163f990006f279a0eaf0e5 Mon Sep 17 00:00:00 2001 From: QuietMisdreavus Date: Thu, 1 Nov 2018 11:57:29 -0500 Subject: [PATCH 7/8] buffer errors from initial tokenization when parsing --- src/librustdoc/test.rs | 15 +++++++--- src/libsyntax/parse/lexer/mod.rs | 23 ++++++++++++++- src/libsyntax/parse/mod.rs | 48 ++++++++++++++++++++++++++++++-- 3 files changed, 79 insertions(+), 7 deletions(-) diff --git a/src/librustdoc/test.rs b/src/librustdoc/test.rs index 442e9bf6b9363..06cb4fbd71602 100644 --- a/src/librustdoc/test.rs +++ b/src/librustdoc/test.rs @@ -422,13 +422,20 @@ pub fn make_test(s: &str, debug!("about to parse: \n{}", source); - // FIXME(misdreavus): this can still emit a FatalError (and thus halt rustdoc prematurely) - // if there is a lexing error in the first token - let mut parser = parse::new_parser_from_source_str(&sess, filename, source); - let mut found_main = false; let mut found_extern_crate = cratename.is_none(); + let mut parser = match parse::maybe_new_parser_from_source_str(&sess, filename, source) { + Ok(p) => p, + Err(errs) => { + for mut err in errs { + err.cancel(); + } + + return (found_main, found_extern_crate); + } + }; + loop { match parser.parse_item() { Ok(Some(item)) => { diff --git a/src/libsyntax/parse/lexer/mod.rs b/src/libsyntax/parse/lexer/mod.rs index a814c88ee7821..dc7a9736b94d8 100644 --- a/src/libsyntax/parse/lexer/mod.rs +++ b/src/libsyntax/parse/lexer/mod.rs @@ -11,7 +11,7 @@ use ast::{self, Ident}; use syntax_pos::{self, BytePos, CharPos, Pos, Span, NO_EXPANSION}; use source_map::{SourceMap, FilePathMapping}; -use errors::{Applicability, FatalError, DiagnosticBuilder}; +use errors::{Applicability, FatalError, Diagnostic, DiagnosticBuilder}; use parse::{token, ParseSess}; use str::char_at; use symbol::{Symbol, keywords}; @@ -175,6 +175,16 @@ impl<'a> StringReader<'a> { self.fatal_errs.clear(); } + pub fn buffer_fatal_errors(&mut self) -> Vec { + let mut buffer = Vec::new(); + + for err in self.fatal_errs.drain(..) { + err.buffer(&mut buffer); + } + + buffer + } + pub fn peek(&self) -> TokenAndSpan { // FIXME(pcwalton): Bad copy! TokenAndSpan { @@ -251,6 +261,17 @@ impl<'a> StringReader<'a> { Ok(sr) } + pub fn new_or_buffered_errs(sess: &'a ParseSess, + source_file: Lrc, + override_span: Option) -> Result> { + let mut sr = StringReader::new_raw(sess, source_file, override_span); + if sr.advance_token().is_err() { + Err(sr.buffer_fatal_errors()) + } else { + Ok(sr) + } + } + pub fn retokenize(sess: &'a ParseSess, mut span: Span) -> Self { let begin = sess.source_map().lookup_byte_offset(span.lo()); let end = sess.source_map().lookup_byte_offset(span.hi()); diff --git a/src/libsyntax/parse/mod.rs b/src/libsyntax/parse/mod.rs index 77a2ae6acf00b..5723c60e87404 100644 --- a/src/libsyntax/parse/mod.rs +++ b/src/libsyntax/parse/mod.rs @@ -15,7 +15,7 @@ use ast::{self, CrateConfig, NodeId}; use early_buffered_lints::{BufferedEarlyLint, BufferedEarlyLintId}; use source_map::{SourceMap, FilePathMapping}; use syntax_pos::{Span, SourceFile, FileName, MultiSpan}; -use errors::{Handler, ColorConfig, DiagnosticBuilder}; +use errors::{Handler, ColorConfig, Diagnostic, DiagnosticBuilder}; use feature_gate::UnstableFeatures; use parse::parser::Parser; use ptr::P; @@ -174,7 +174,7 @@ pub fn parse_stream_from_source_str(name: FileName, source: String, sess: &Parse source_file_to_stream(sess, sess.source_map().new_source_file(name, source), override_span) } -// Create a new parser from a source string +/// Create a new parser from a source string pub fn new_parser_from_source_str(sess: &ParseSess, name: FileName, source: String) -> Parser { let mut parser = source_file_to_parser(sess, sess.source_map().new_source_file(name, source)); @@ -182,6 +182,17 @@ pub fn new_parser_from_source_str(sess: &ParseSess, name: FileName, source: Stri parser } +/// Create a new parser from a source string. Returns any buffered errors from lexing the initial +/// token stream. +pub fn maybe_new_parser_from_source_str(sess: &ParseSess, name: FileName, source: String) + -> Result> +{ + let mut parser = maybe_source_file_to_parser(sess, + sess.source_map().new_source_file(name, source))?; + parser.recurse_into_file_modules = false; + Ok(parser) +} + /// Create a new parser, handling errors as appropriate /// if the file doesn't exist pub fn new_parser_from_file<'a>(sess: &'a ParseSess, path: &Path) -> Parser<'a> { @@ -214,6 +225,21 @@ fn source_file_to_parser(sess: & ParseSess, source_file: Lrc) -> Par parser } +/// Given a source_file and config, return a parser. Returns any buffered errors from lexing the +/// initial token stream. +fn maybe_source_file_to_parser(sess: &ParseSess, source_file: Lrc) + -> Result> +{ + let end_pos = source_file.end_pos; + let mut parser = stream_to_parser(sess, maybe_file_to_stream(sess, source_file, None)?); + + if parser.token == token::Eof && parser.span.is_dummy() { + parser.span = Span::new(end_pos, end_pos, parser.span.ctxt()); + } + + Ok(parser) +} + // must preserve old name for now, because quote! from the *existing* // compiler expands into it pub fn new_parser_from_tts(sess: &ParseSess, tts: Vec) -> Parser { @@ -248,6 +274,24 @@ pub fn source_file_to_stream(sess: &ParseSess, panictry!(srdr.parse_all_token_trees()) } +/// Given a source file, produce a sequence of token-trees. Returns any buffered errors from +/// parsing the token tream. +pub fn maybe_file_to_stream(sess: &ParseSess, + source_file: Lrc, + override_span: Option) -> Result> { + let mut srdr = lexer::StringReader::new_or_buffered_errs(sess, source_file, override_span)?; + srdr.real_token(); + + match srdr.parse_all_token_trees() { + Ok(stream) => Ok(stream), + Err(err) => { + let mut buffer = Vec::with_capacity(1); + err.buffer(&mut buffer); + Err(buffer) + } + } +} + /// Given stream and the `ParseSess`, produce a parser pub fn stream_to_parser(sess: &ParseSess, stream: TokenStream) -> Parser { Parser::new(sess, stream, None, true, false) From 014c8c4c3872ff74169ffbbc3a69acd92be2a76c Mon Sep 17 00:00:00 2001 From: QuietMisdreavus Date: Thu, 1 Nov 2018 16:01:38 -0500 Subject: [PATCH 8/8] implement existing parser fns in terms of fallible fns --- src/libsyntax/lib.rs | 17 +++++++++++++++++ src/libsyntax/parse/mod.rs | 18 ++++-------------- 2 files changed, 21 insertions(+), 14 deletions(-) diff --git a/src/libsyntax/lib.rs b/src/libsyntax/lib.rs index 9077eca18215c..e9a6535cba1d2 100644 --- a/src/libsyntax/lib.rs +++ b/src/libsyntax/lib.rs @@ -70,6 +70,23 @@ macro_rules! panictry { }) } +// A variant of 'panictry!' that works on a Vec instead of a single DiagnosticBuilder. +macro_rules! panictry_buffer { + ($handler:expr, $e:expr) => ({ + use std::result::Result::{Ok, Err}; + use errors::{FatalError, DiagnosticBuilder}; + match $e { + Ok(e) => e, + Err(errs) => { + for e in errs { + DiagnosticBuilder::new_diagnostic($handler, e).emit(); + } + FatalError.raise() + } + } + }) +} + #[macro_export] macro_rules! unwrap_or { ($opt:expr, $default:expr) => { diff --git a/src/libsyntax/parse/mod.rs b/src/libsyntax/parse/mod.rs index 5723c60e87404..8b2020c64189c 100644 --- a/src/libsyntax/parse/mod.rs +++ b/src/libsyntax/parse/mod.rs @@ -177,9 +177,7 @@ pub fn parse_stream_from_source_str(name: FileName, source: String, sess: &Parse /// Create a new parser from a source string pub fn new_parser_from_source_str(sess: &ParseSess, name: FileName, source: String) -> Parser { - let mut parser = source_file_to_parser(sess, sess.source_map().new_source_file(name, source)); - parser.recurse_into_file_modules = false; - parser + panictry_buffer!(&sess.span_diagnostic, maybe_new_parser_from_source_str(sess, name, source)) } /// Create a new parser from a source string. Returns any buffered errors from lexing the initial @@ -215,14 +213,8 @@ crate fn new_sub_parser_from_file<'a>(sess: &'a ParseSess, /// Given a source_file and config, return a parser fn source_file_to_parser(sess: & ParseSess, source_file: Lrc) -> Parser { - let end_pos = source_file.end_pos; - let mut parser = stream_to_parser(sess, source_file_to_stream(sess, source_file, None)); - - if parser.token == token::Eof && parser.span.is_dummy() { - parser.span = Span::new(end_pos, end_pos, parser.span.ctxt()); - } - - parser + panictry_buffer!(&sess.span_diagnostic, + maybe_source_file_to_parser(sess, source_file)) } /// Given a source_file and config, return a parser. Returns any buffered errors from lexing the @@ -269,9 +261,7 @@ fn file_to_source_file(sess: &ParseSess, path: &Path, spanopt: Option) pub fn source_file_to_stream(sess: &ParseSess, source_file: Lrc, override_span: Option) -> TokenStream { - let mut srdr = lexer::StringReader::new(sess, source_file, override_span); - srdr.real_token(); - panictry!(srdr.parse_all_token_trees()) + panictry_buffer!(&sess.span_diagnostic, maybe_file_to_stream(sess, source_file, override_span)) } /// Given a source file, produce a sequence of token-trees. Returns any buffered errors from