]> git.lizzy.rs Git - rust.git/commitdiff
make codemap more robust in face of ill-formed spans.
authorFelix S. Klock II <pnkfelix@pnkfx.org>
Thu, 5 Feb 2015 15:02:22 +0000 (16:02 +0100)
committerFelix S. Klock II <pnkfelix@pnkfx.org>
Thu, 5 Feb 2015 22:47:17 +0000 (23:47 +0100)
This can be considered partial work on #8256.

The main observable change: macro expansion sometimes results in spans
where `lo > hi`; so for now, when we have such a span, do not attempt
to return a snippet result.

(Longer term, we might think about whether we could still present a
snippet for the cases where this arises, e.g. perhaps by showing the
whole macro as the snippet, assuming that is the sole cause of such
spans; or by somehow looking up the closest AST node that holds both
`lo` and `hi`, and showing that.)

As a drive-by, revised the API to return a `Result` rather than an
`Option`, with better information-packed error value that should help
us (and maybe also our users) identify the causes of such problems in
the future.  Ideally the call-sites that really want an actual snippet
would be updated to catch the newly added `Err` case and print
something meaningful about it, but that is not part of this PR.

src/librustc_trans/save/span_utils.rs
src/librustc_trans/trans/debuginfo.rs
src/librustdoc/clean/mod.rs
src/libsyntax/codemap.rs
src/libsyntax/parse/mod.rs

index beec8071a72baf4f1491864da078b072aae0ca28..a724cdc0229d2f465af8865f8cd1ae42951dda33 100644 (file)
@@ -69,8 +69,8 @@ pub fn make_sub_span(&self, span: Span, sub_span: Option<Span>) -> Option<Span>
 
     pub fn snippet(&self, span: Span) -> String {
         match self.sess.codemap().span_to_snippet(span) {
-            Some(s) => s,
-            None => String::new(),
+            Ok(s) => s,
+            Err(_) => String::new(),
         }
     }
 
index 1e788351172fc0e2ad26c00d6eccfc57915ca459..4d4a2bf48548abf260677929750f807e28ec4881 100644 (file)
@@ -1094,7 +1094,7 @@ pub fn get_cleanup_debug_loc_for_ast_node<'a, 'tcx>(cx: &CrateContext<'a, 'tcx>,
         // bodies), in which case we also just want to return the span of the
         // whole expression.
         let code_snippet = cx.sess().codemap().span_to_snippet(node_span);
-        if let Some(code_snippet) = code_snippet {
+        if let Ok(code_snippet) = code_snippet {
             let bytes = code_snippet.as_bytes();
 
             if bytes.len() > 0 && &bytes[bytes.len()-1..] == b"}" {
index 248ce99ff9b724a453269d88283fea87ce9bdb65..611251d4cfae114d895992a24def11931a680168 100644 (file)
@@ -2301,8 +2301,8 @@ impl ToSource for syntax::codemap::Span {
     fn to_src(&self, cx: &DocContext) -> String {
         debug!("converting span {:?} to snippet", self.clean(cx));
         let sn = match cx.sess().codemap().span_to_snippet(*self) {
-            Some(x) => x.to_string(),
-            None    => "".to_string()
+            Ok(x) => x.to_string(),
+            Err(_) => "".to_string()
         };
         debug!("got snippet {}", sn);
         sn
index 00857d10f439e763e4cdb3af4cbf160d09f0fc39..3231342cb50c8f37524ca7d44ee66fb163d56344 100644 (file)
@@ -437,18 +437,35 @@ pub fn span_to_lines(&self, sp: Span) -> FileLines {
         FileLines {file: lo.file, lines: lines}
     }
 
-    pub fn span_to_snippet(&self, sp: Span) -> Option<String> {
+    pub fn span_to_snippet(&self, sp: Span) -> Result<String, SpanSnippetError> {
+        if sp.lo > sp.hi {
+            return Err(SpanSnippetError::IllFormedSpan(sp));
+        }
+
         let begin = self.lookup_byte_offset(sp.lo);
         let end = self.lookup_byte_offset(sp.hi);
 
-        // FIXME #8256: this used to be an assert but whatever precondition
-        // it's testing isn't true for all spans in the AST, so to allow the
-        // caller to not have to panic (and it can't catch it since the CodeMap
-        // isn't sendable), return None
         if begin.fm.start_pos != end.fm.start_pos {
-            None
+            return Err(SpanSnippetError::DistinctSources(DistinctSources {
+                begin: (begin.fm.name.clone(),
+                        begin.fm.start_pos),
+                end: (end.fm.name.clone(),
+                      end.fm.start_pos)
+            }));
         } else {
-            Some((&begin.fm.src[begin.pos.to_usize()..end.pos.to_usize()]).to_string())
+            let start = begin.pos.to_usize();
+            let limit = end.pos.to_usize();
+            if start > limit || limit > begin.fm.src.len() {
+                return Err(SpanSnippetError::MalformedForCodemap(
+                    MalformedCodemapPositions {
+                        name: begin.fm.name.clone(),
+                        source_len: begin.fm.src.len(),
+                        begin_pos: begin.pos,
+                        end_pos: end.pos,
+                    }));
+            }
+
+            return Ok((&begin.fm.src[start..limit]).to_string())
         }
     }
 
@@ -622,6 +639,27 @@ pub fn span_is_internal(&self, span: Span) -> bool {
     }
 }
 
+#[derive(Clone, PartialEq, Eq, Debug)]
+pub enum SpanSnippetError {
+    IllFormedSpan(Span),
+    DistinctSources(DistinctSources),
+    MalformedForCodemap(MalformedCodemapPositions),
+}
+
+#[derive(Clone, PartialEq, Eq, Debug)]
+pub struct DistinctSources {
+    begin: (String, BytePos),
+    end: (String, BytePos)
+}
+
+#[derive(Clone, PartialEq, Eq, Debug)]
+pub struct MalformedCodemapPositions {
+    name: String,
+    source_len: usize,
+    begin_pos: BytePos,
+    end_pos: BytePos
+}
+
 #[cfg(test)]
 mod test {
     use super::*;
@@ -773,7 +811,7 @@ fn t8() {
         let span = Span {lo: BytePos(12), hi: BytePos(23), expn_id: NO_EXPANSION};
         let snippet = cm.span_to_snippet(span);
 
-        assert_eq!(snippet, Some("second line".to_string()));
+        assert_eq!(snippet, Ok("second line".to_string()));
     }
 
     #[test]
index 6ff5c77f5079a954aa4a1314cee858953ff369a3..9e9ec08da038e68855df48ebc1332b010184513d 100644 (file)
@@ -1233,8 +1233,8 @@ fn ttdelim_span() {
         let span = tts.iter().rev().next().unwrap().get_span();
 
         match sess.span_diagnostic.cm.span_to_snippet(span) {
-            Some(s) => assert_eq!(&s[], "{ body }"),
-            None => panic!("could not get snippet"),
+            Ok(s) => assert_eq!(&s[], "{ body }"),
+            Err(_) => panic!("could not get snippet"),
         }
     }
 }