From 49d2fd1725510fd3bf6f2937e178b1aa055ddb02 Mon Sep 17 00:00:00 2001 From: Alexander Regueiro Date: Fri, 6 Sep 2019 03:56:45 +0100 Subject: Aggregation of cosmetic changes made during work on REPL PRs: libsyntax --- src/libsyntax/source_map.rs | 128 ++++++++++++++++++++++---------------------- 1 file changed, 65 insertions(+), 63 deletions(-) (limited to 'src/libsyntax/source_map.rs') diff --git a/src/libsyntax/source_map.rs b/src/libsyntax/source_map.rs index 7190cfd72a9..393723b02b2 100644 --- a/src/libsyntax/source_map.rs +++ b/src/libsyntax/source_map.rs @@ -1,10 +1,10 @@ -//! The SourceMap tracks all the source code used within a single crate, mapping +//! The `SourceMap` tracks all the source code used within a single crate, mapping //! from integer byte positions to the original source code location. Each bit //! of source parsed during crate parsing (typically files, in-memory strings, //! or various bits of macro expansion) cover a continuous range of bytes in the -//! SourceMap and are represented by SourceFiles. Byte positions are stored in -//! `spans` and used pervasively in the compiler. They are absolute positions -//! within the SourceMap, which upon request can be converted to line and column +//! `SourceMap` and are represented by `SourceFile`s. Byte positions are stored in +//! `Span`` and used pervasively in the compiler. They are absolute positions +//! within the `SourceMap`, which upon request can be converted to line and column //! information, source code snippets, etc. pub use syntax_pos::*; @@ -94,7 +94,7 @@ impl FileLoader for RealFileLoader { } } -// This is a SourceFile identifier that is used to correlate SourceFiles between +// This is a `SourceFile` identifier that is used to correlate `SourceFile`s between // subsequent compilation sessions (which is something we need to do during // incremental compilation). #[derive(Copy, Clone, PartialEq, Eq, Hash, RustcEncodable, RustcDecodable, Debug)] @@ -103,8 +103,8 @@ pub struct StableSourceFileId(u128); impl StableSourceFileId { pub fn new(source_file: &SourceFile) -> StableSourceFileId { StableSourceFileId::new_from_pieces(&source_file.name, - source_file.name_was_remapped, - source_file.unmapped_path.as_ref()) + source_file.name_was_remapped, + source_file.unmapped_path.as_ref()) } pub fn new_from_pieces(name: &FileName, @@ -134,7 +134,7 @@ pub struct SourceMap { files: Lock, file_loader: Box, // This is used to apply the file path remapping as specified via - // --remap-path-prefix to all SourceFiles allocated within this SourceMap. + // `--remap-path-prefix` to all `SourceFile`s allocated within this `SourceMap`. path_mapping: FilePathMapping, } @@ -204,14 +204,14 @@ impl SourceMap { match self.files.borrow().source_files.last() { None => 0, // Add one so there is some space between files. This lets us distinguish - // positions in the source_map, even in the presence of zero-length files. + // positions in the `SourceMap`, even in the presence of zero-length files. Some(last) => last.end_pos.to_usize() + 1, } } - /// Creates a new source_file. - /// If a file already exists in the source_map with the same id, that file is returned - /// unmodified + /// Creates a new `SourceFile`. + /// If a file already exists in the `SourceMap` with the same ID, that file is returned + /// unmodified. pub fn new_source_file(&self, filename: FileName, src: String) -> Lrc { self.try_new_source_file(filename, src) .unwrap_or_else(|OffsetOverflowError| { @@ -268,8 +268,8 @@ impl SourceMap { Ok(lrc_sf) } - /// Allocates a new SourceFile representing a source file from an external - /// crate. The source code of such an "imported source_file" is not available, + /// Allocates a new `SourceFile` representing a source file from an external + /// crate. The source code of such an "imported `SourceFile`" is not available, /// but we still know enough to generate accurate debuginfo location /// information for things inlined from other crates. pub fn new_imported_source_file( @@ -334,7 +334,7 @@ impl SourceMap { pos.col.to_usize() + 1) } - // If there is a doctest_offset, apply it to the line + // If there is a doctest offset, applies it to the line. pub fn doctest_offset_line(&self, file: &FileName, orig: usize) -> usize { return match file { FileName::DocTest(_, offset) => { @@ -348,7 +348,7 @@ impl SourceMap { } } - /// Lookup source information about a BytePos + /// Looks up source information about a `BytePos`. pub fn lookup_char_pos(&self, pos: BytePos) -> Loc { let chpos = self.bytepos_to_file_charpos(pos); match self.lookup_line(pos) { @@ -411,7 +411,7 @@ impl SourceMap { } } - // If the relevant source_file is empty, we don't return a line number. + // If the corresponding `SourceFile` is empty, does not return a line number. pub fn lookup_line(&self, pos: BytePos) -> Result> { let idx = self.lookup_source_file_idx(pos); @@ -423,15 +423,15 @@ impl SourceMap { } } - /// Returns `Some(span)`, a union of the lhs and rhs span. The lhs must precede the rhs. If - /// there are gaps between lhs and rhs, the resulting union will cross these gaps. - /// For this to work, the spans have to be: + /// Returns `Some(span)`, a union of the LHS and RHS span. The LHS must precede the RHS. If + /// there are gaps between LHS and RHS, the resulting union will cross these gaps. + /// For this to work, /// - /// * the ctxt of both spans much match - /// * the lhs span needs to end on the same line the rhs span begins - /// * the lhs span must start at or before the rhs span + /// * the syntax contexts of both spans much match, + /// * the LHS span needs to end on the same line the RHS span begins, + /// * the LHS span must start at or before the RHS span. pub fn merge_spans(&self, sp_lhs: Span, sp_rhs: Span) -> Option { - // make sure we're at the same expansion id + // Ensure we're at the same expansion ID. if sp_lhs.ctxt() != sp_rhs.ctxt() { return None; } @@ -445,12 +445,12 @@ impl SourceMap { Err(_) => return None }; - // if we must cross lines to merge, don't merge + // If we must cross lines to merge, don't merge. if lhs_end.line != rhs_begin.line { return None; } - // ensure these follow the expected order and we don't overlap + // Ensure these follow the expected order and that we don't overlap. if (sp_lhs.lo() <= sp_rhs.lo()) && (sp_lhs.hi() <= sp_rhs.lo()) { Some(sp_lhs.to(sp_rhs)) } else { @@ -466,11 +466,12 @@ impl SourceMap { let lo = self.lookup_char_pos(sp.lo()); let hi = self.lookup_char_pos(sp.hi()); format!("{}:{}:{}: {}:{}", - lo.file.name, - lo.line, - lo.col.to_usize() + 1, - hi.line, - hi.col.to_usize() + 1) + lo.file.name, + lo.line, + lo.col.to_usize() + 1, + hi.line, + hi.col.to_usize() + 1, + ) } pub fn span_to_filename(&self, sp: Span) -> FileName { @@ -478,8 +479,8 @@ impl SourceMap { } pub fn span_to_unmapped_path(&self, sp: Span) -> FileName { - self.lookup_char_pos(sp.lo()).file.unmapped_path.clone() - .expect("SourceMap::span_to_unmapped_path called for imported SourceFile?") + let source_file = self.lookup_char_pos(sp.lo()).file; + source_file.unmapped_path.clone().unwrap_or(source_file.name.clone()) } pub fn is_multiline(&self, sp: Span) -> bool { @@ -586,7 +587,7 @@ impl SourceMap { } } - /// Returns the source snippet as `String` corresponding to the given `Span` + /// Returns the source snippet as `String` corresponding to the given `Span`. pub fn span_to_snippet(&self, sp: Span) -> Result { self.span_to_source(sp, |src, start_index, end_index| src.get(start_index..end_index) .map(|s| s.to_string()) @@ -602,14 +603,14 @@ impl SourceMap { } } - /// Returns the source snippet as `String` before the given `Span` + /// Returns the source snippet as `String` before the given `Span`. pub fn span_to_prev_source(&self, sp: Span) -> Result { self.span_to_source(sp, |src, start_index, _| src.get(..start_index) .map(|s| s.to_string()) .ok_or_else(|| SpanSnippetError::IllFormedSpan(sp))) } - /// Extend the given `Span` to just after the previous occurrence of `c`. Return the same span + /// Extends the given `Span` to just after the previous occurrence of `c`. Return the same span /// if no character could be found or if an error occurred while retrieving the code snippet. pub fn span_extend_to_prev_char(&self, sp: Span, c: char) -> Span { if let Ok(prev_source) = self.span_to_prev_source(sp) { @@ -622,8 +623,8 @@ impl SourceMap { sp } - /// Extend the given `Span` to just after the previous occurrence of `pat` when surrounded by - /// whitespace. Return the same span if no character could be found or if an error occurred + /// Extends the given `Span` to just after the previous occurrence of `pat` when surrounded by + /// whitespace. Returns the same span if no character could be found or if an error occurred /// while retrieving the code snippet. pub fn span_extend_to_prev_str(&self, sp: Span, pat: &str, accept_newlines: bool) -> Span { // assure that the pattern is delimited, to avoid the following @@ -643,7 +644,8 @@ impl SourceMap { sp } - /// Given a `Span`, try to get a shorter span ending before the first occurrence of `c` `char` + /// Given a `Span`, tries to get a shorter span ending before the first occurrence of `char` + /// ``c`. pub fn span_until_char(&self, sp: Span, c: char) -> Span { match self.span_to_snippet(sp) { Ok(snippet) => { @@ -658,7 +660,7 @@ impl SourceMap { } } - /// Given a `Span`, try to get a shorter span ending just after the first occurrence of `char` + /// Given a `Span`, tries to get a shorter span ending just after the first occurrence of `char` /// `c`. pub fn span_through_char(&self, sp: Span, c: char) -> Span { if let Ok(snippet) = self.span_to_snippet(sp) { @@ -669,8 +671,8 @@ impl SourceMap { sp } - /// Given a `Span`, get a new `Span` covering the first token and all its trailing whitespace or - /// the original `Span`. + /// Given a `Span`, gets a new `Span` covering the first token and all its trailing whitespace + /// or the original `Span`. /// /// If `sp` points to `"let mut x"`, then a span pointing at `"let "` will be returned. pub fn span_until_non_whitespace(&self, sp: Span) -> Span { @@ -689,15 +691,15 @@ impl SourceMap { }) } - /// Given a `Span`, get a new `Span` covering the first token without its trailing whitespace or - /// the original `Span` in case of error. + /// Given a `Span`, gets a new `Span` covering the first token without its trailing whitespace + /// or the original `Span` in case of error. /// /// If `sp` points to `"let mut x"`, then a span pointing at `"let"` will be returned. pub fn span_until_whitespace(&self, sp: Span) -> Span { self.span_take_while(sp, |c| !c.is_whitespace()) } - /// Given a `Span`, get a shorter one until `predicate` yields false. + /// Given a `Span`, gets a shorter one until `predicate` yields `false`. pub fn span_take_while

(&self, sp: Span, predicate: P) -> Span where P: for <'r> FnMut(&'r char) -> bool { @@ -717,7 +719,7 @@ impl SourceMap { self.span_until_char(sp, '{') } - /// Returns a new span representing just the start-point of this span + /// Returns a new span representing just the start point of this span. pub fn start_point(&self, sp: Span) -> Span { let pos = sp.lo().0; let width = self.find_width_of_character_at_span(sp, false); @@ -726,7 +728,7 @@ impl SourceMap { sp.with_hi(end_point) } - /// Returns a new span representing just the end-point of this span + /// Returns a new span representing just the end point of this span. pub fn end_point(&self, sp: Span) -> Span { let pos = sp.hi().0; @@ -737,7 +739,7 @@ impl SourceMap { sp.with_lo(end_point) } - /// Returns a new span representing the next character after the end-point of this span + /// Returns a new span representing the next character after the end-point of this span. pub fn next_point(&self, sp: Span) -> Span { let start_of_next_point = sp.hi().0; @@ -840,7 +842,7 @@ impl SourceMap { None } - /// For a global BytePos compute the local offset within the containing SourceFile + /// For a global `BytePos`, computes the local offset within the containing `SourceFile`. pub fn lookup_byte_offset(&self, bpos: BytePos) -> SourceFileAndBytePos { let idx = self.lookup_source_file_idx(bpos); let sf = (*self.files.borrow().source_files)[idx].clone(); @@ -848,22 +850,22 @@ impl SourceMap { SourceFileAndBytePos {sf, pos: offset} } - /// Converts an absolute BytePos to a CharPos relative to the source_file. + /// Converts an absolute `BytePos` to a `CharPos` relative to the `SourceFile`. pub fn bytepos_to_file_charpos(&self, bpos: BytePos) -> CharPos { let idx = self.lookup_source_file_idx(bpos); let map = &(*self.files.borrow().source_files)[idx]; - // The number of extra bytes due to multibyte chars in the SourceFile + // The number of extra bytes due to multibyte chars in the `SourceFile`. let mut total_extra_bytes = 0; for mbc in map.multibyte_chars.iter() { debug!("{}-byte char at {:?}", mbc.bytes, mbc.pos); if mbc.pos < bpos { - // every character is at least one byte, so we only + // Every character is at least one byte, so we only // count the actual extra bytes. total_extra_bytes += mbc.bytes as u32 - 1; // We should never see a byte position in the middle of a - // character + // character. assert!(bpos.to_u32() >= mbc.pos.to_u32() + mbc.bytes as u32); } else { break; @@ -874,13 +876,13 @@ impl SourceMap { CharPos(bpos.to_usize() - map.start_pos.to_usize() - total_extra_bytes as usize) } - // Return the index of the source_file (in self.files) which contains pos. + // Returns the index of the `SourceFile` (in `self.files`) that contains `pos`. pub fn lookup_source_file_idx(&self, pos: BytePos) -> usize { let files = self.files.borrow(); let files = &files.source_files; let count = files.len(); - // Binary search for the source_file. + // Binary search for the `SourceFile`. let mut a = 0; let mut b = count; while b - a > 1 { @@ -911,8 +913,8 @@ impl SourceMap { }).ok() } - /// Take the span of a type parameter in a function signature and try to generate a span for the - /// function name (with generics) and a new snippet for this span with the pointed type + /// Takes the span of a type parameter in a function signature and try to generate a span for + /// the function name (with generics) and a new snippet for this span with the pointed type /// parameter as a new local type parameter. /// /// For instance: @@ -928,18 +930,18 @@ impl SourceMap { /// /// Attention: The method used is very fragile since it essentially duplicates the work of the /// parser. If you need to use this function or something similar, please consider updating the - /// source_map functions and this function to something more robust. + /// `SourceMap` functions and this function to something more robust. pub fn generate_local_type_param_snippet(&self, span: Span) -> Option<(Span, String)> { // Try to extend the span to the previous "fn" keyword to retrieve the function - // signature + // signature. let sugg_span = self.span_extend_to_prev_str(span, "fn", false); if sugg_span != span { if let Ok(snippet) = self.span_to_snippet(sugg_span) { - // Consume the function name + // Consume the function name. let mut offset = snippet.find(|c: char| !c.is_alphanumeric() && c != '_') .expect("no label after fn"); - // Consume the generics part of the function signature + // Consume the generics part of the function signature. let mut bracket_counter = 0; let mut last_char = None; for c in snippet[offset..].chars() { @@ -953,11 +955,11 @@ impl SourceMap { last_char = Some(c); } - // Adjust the suggestion span to encompass the function name with its generics + // Adjust the suggestion span to encompass the function name with its generics. let sugg_span = sugg_span.with_hi(BytePos(sugg_span.lo().0 + offset as u32)); // Prepare the new suggested snippet to append the type parameter that triggered - // the error in the generics of the function signature + // the error in the generics of the function signature. let mut new_snippet = if last_char == Some('>') { format!("{}, ", &snippet[..(offset - '>'.len_utf8())]) } else { -- cgit 1.4.1-3-g733a5 From 553a56dd98cee2e42fe8de0e3ad02a41a4a0eb13 Mon Sep 17 00:00:00 2001 From: Alexander Regueiro Date: Fri, 6 Sep 2019 22:38:07 +0100 Subject: Apply suggestions from code review --- src/libsyntax/parse/diagnostics.rs | 2 +- src/libsyntax/parse/lexer/tests.rs | 2 +- src/libsyntax/parse/parser/expr.rs | 10 +++++----- src/libsyntax/parse/parser/stmt.rs | 2 +- src/libsyntax/source_map.rs | 4 ++-- src/libsyntax/source_map/tests.rs | 13 ++++++------- src/libsyntax/visit.rs | 2 +- 7 files changed, 17 insertions(+), 18 deletions(-) (limited to 'src/libsyntax/source_map.rs') diff --git a/src/libsyntax/parse/diagnostics.rs b/src/libsyntax/parse/diagnostics.rs index 2890a8e721e..3120d0e3517 100644 --- a/src/libsyntax/parse/diagnostics.rs +++ b/src/libsyntax/parse/diagnostics.rs @@ -1086,7 +1086,7 @@ impl<'a> Parser<'a> { /// statement. This is something of a best-effort heuristic. /// /// We terminate when we find an unmatched `}` (without consuming it). - pub fn recover_stmt(&mut self) { + crate fn recover_stmt(&mut self) { self.recover_stmt_(SemiColonMode::Ignore, BlockMode::Ignore) } diff --git a/src/libsyntax/parse/lexer/tests.rs b/src/libsyntax/parse/lexer/tests.rs index d965bf28ee7..c1ec41902e2 100644 --- a/src/libsyntax/parse/lexer/tests.rs +++ b/src/libsyntax/parse/lexer/tests.rs @@ -39,7 +39,7 @@ fn t1() { let mut string_reader = setup( &sm, &sh, - "/* my source file */ fn main() { println!(\"zebra\"); }\n".to_owned(), + "/* my source file */ fn main() { println!(\"zebra\"); }\n".to_string(), ); assert_eq!(string_reader.next_token(), token::Comment); assert_eq!(string_reader.next_token(), token::Whitespace); diff --git a/src/libsyntax/parse/parser/expr.rs b/src/libsyntax/parse/parser/expr.rs index f70c607198f..4dbb5ff75eb 100644 --- a/src/libsyntax/parse/parser/expr.rs +++ b/src/libsyntax/parse/parser/expr.rs @@ -843,7 +843,7 @@ impl<'a> Parser<'a> { return self.parse_block_expr(None, lo, BlockCheckMode::Default, attrs); } token::BinOp(token::Or) | token::OrOr => { - return self.parse_closure(attrs); + return self.parse_closure_expr(attrs); } token::OpenDelim(token::Bracket) => { self.bump(); @@ -919,7 +919,7 @@ impl<'a> Parser<'a> { return self.maybe_recover_from_bad_qpath(expr, true); } if self.check_keyword(kw::Move) || self.check_keyword(kw::Static) { - return self.parse_closure(attrs); + return self.parse_closure_expr(attrs); } if self.eat_keyword(kw::If) { return self.parse_if_expr(attrs); @@ -996,7 +996,7 @@ impl<'a> Parser<'a> { return if self.is_async_block() { // Check for `async {` and `async move {`. self.parse_async_block(attrs) } else { - self.parse_closure(attrs) + self.parse_closure_expr(attrs) }; } if self.eat_keyword(kw::Return) { @@ -1097,8 +1097,8 @@ impl<'a> Parser<'a> { Ok(self.mk_expr(blk.span, ExprKind::Block(blk, opt_label), attrs)) } - /// Parses a closure (e.g., `move |args| expr`). - fn parse_closure(&mut self, attrs: ThinVec) -> PResult<'a, P> { + /// Parses a closure expression (e.g., `move |args| expr`). + fn parse_closure_expr(&mut self, attrs: ThinVec) -> PResult<'a, P> { let lo = self.token.span; let movability = if self.eat_keyword(kw::Static) { diff --git a/src/libsyntax/parse/parser/stmt.rs b/src/libsyntax/parse/parser/stmt.rs index 6a3ac2d73f8..04bd61a4cfb 100644 --- a/src/libsyntax/parse/parser/stmt.rs +++ b/src/libsyntax/parse/parser/stmt.rs @@ -422,7 +422,7 @@ impl<'a> Parser<'a> { } /// Parses a statement, including the trailing semicolon. - pub fn parse_full_stmt(&mut self, macro_legacy_warnings: bool) -> PResult<'a, Option> { + crate fn parse_full_stmt(&mut self, macro_legacy_warnings: bool) -> PResult<'a, Option> { // Skip looking for a trailing semicolon when we have an interpolated statement. maybe_whole!(self, NtStmt, |x| Some(x)); diff --git a/src/libsyntax/source_map.rs b/src/libsyntax/source_map.rs index 393723b02b2..d7ea799e004 100644 --- a/src/libsyntax/source_map.rs +++ b/src/libsyntax/source_map.rs @@ -479,8 +479,8 @@ impl SourceMap { } pub fn span_to_unmapped_path(&self, sp: Span) -> FileName { - let source_file = self.lookup_char_pos(sp.lo()).file; - source_file.unmapped_path.clone().unwrap_or(source_file.name.clone()) + self.lookup_char_pos(sp.lo()).file.unmapped_path.clone() + .expect("`SourceMap::span_to_unmapped_path` called for imported `SourceFile`?") } pub fn is_multiline(&self, sp: Span) -> bool { diff --git a/src/libsyntax/source_map/tests.rs b/src/libsyntax/source_map/tests.rs index 28fc1909324..15254336bbf 100644 --- a/src/libsyntax/source_map/tests.rs +++ b/src/libsyntax/source_map/tests.rs @@ -97,7 +97,7 @@ fn t6() { #[test] fn t7() { let sm = init_source_map(); - let span = Span::new(BytePos(12), BytePos(23), NO_EXPANSION); + let span = Span::with_root_ctxt(BytePos(12), BytePos(23)); let file_lines = sm.span_to_lines(span).unwrap(); assert_eq!(file_lines.file.name, PathBuf::from("blork.rs").into()); @@ -113,7 +113,7 @@ fn span_from_selection(input: &str, selection: &str) -> Span { assert_eq!(input.len(), selection.len()); let left_index = selection.find('~').unwrap() as u32; let right_index = selection.rfind('~').map(|x|x as u32).unwrap_or(left_index); - Span::new(BytePos(left_index), BytePos(right_index + 1), NO_EXPANSION) + Span::with_root_ctxt(BytePos(left_index), BytePos(right_index + 1)) } /// Tests `span_to_snippet` and `span_to_lines` for a span converting 3 @@ -143,7 +143,7 @@ fn span_to_snippet_and_lines_spanning_multiple_lines() { #[test] fn t8() { let sm = init_source_map(); - let span = Span::new(BytePos(12), BytePos(23), NO_EXPANSION); + let span = Span::with_root_ctxt(BytePos(12), BytePos(23)); let snippet = sm.span_to_snippet(span); assert_eq!(snippet, Ok("second line".to_string())); @@ -153,7 +153,7 @@ fn t8() { #[test] fn t9() { let sm = init_source_map(); - let span = Span::new(BytePos(12), BytePos(23), NO_EXPANSION); + let span = Span::with_root_ctxt(BytePos(12), BytePos(23)); let sstr = sm.span_to_string(span); assert_eq!(sstr, "blork.rs:2:1: 2:12"); @@ -176,7 +176,7 @@ fn span_merging_fail() { /// Returns the span corresponding to the `n`th occurrence of `substring` in `source_text`. trait SourceMapExtension { fn span_substr( - self, + &self, file: &Lrc, source_text: &str, substring: &str, @@ -208,10 +208,9 @@ impl SourceMapExtension for SourceMap { let lo = hi + offset; hi = lo + substring.len(); if i == n { - let span = Span::new( + let span = Span::with_root_ctxt( BytePos(lo as u32 + file.start_pos.0), BytePos(hi as u32 + file.start_pos.0), - NO_EXPANSION, ); assert_eq!(&self.span_to_snippet(span).unwrap()[..], substring); return span; diff --git a/src/libsyntax/visit.rs b/src/libsyntax/visit.rs index 1cff834b7ad..d7c537be896 100644 --- a/src/libsyntax/visit.rs +++ b/src/libsyntax/visit.rs @@ -344,7 +344,6 @@ pub fn walk_ty<'a, V: Visitor<'a>>(visitor: &mut V, typ: &'a Ty) { walk_list!(visitor, visit_lifetime, opt_lifetime); visitor.visit_ty(&mutable_type.ty) } - TyKind::Never => {} TyKind::Tup(ref tuple_element_types) => { walk_list!(visitor, visit_ty, tuple_element_types); } @@ -373,6 +372,7 @@ pub fn walk_ty<'a, V: Visitor<'a>>(visitor: &mut V, typ: &'a Ty) { TyKind::Mac(ref mac) => { visitor.visit_mac(mac) } + TyKind::Never | TyKind::CVarArgs => {} } } -- cgit 1.4.1-3-g733a5