Apply suggestions from code review

Author: Alexander Regueiro

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)
     }
 

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);

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<Attribute>) -> PResult<'a, P<Expr>> {
+    /// Parses a closure expression (e.g., `move |args| expr`).
+    fn parse_closure_expr(&mut self, attrs: ThinVec<Attribute>) -> PResult<'a, P<Expr>> {
         let lo = self.token.span;
 
         let movability = if self.eat_keyword(kw::Static) {

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<Stmt>> {
+    crate fn parse_full_stmt(&mut self, macro_legacy_warnings: bool) -> PResult<'a, Option<Stmt>> {
         // Skip looking for a trailing semicolon when we have an interpolated statement.
         maybe_whole!(self, NtStmt, |x| Some(x));
 

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 {

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<SourceFile>,
         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;

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 => {}
     }
 }