From fa9d2230a65689289f64c897ad30c83251762320 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Thu, 5 Feb 2015 16:02:22 +0100 Subject: [PATCH] make codemap more robust in face of ill-formed spans. 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 | 4 +- src/librustc_trans/trans/debuginfo.rs | 2 +- src/librustdoc/clean/mod.rs | 4 +- src/libsyntax/codemap.rs | 54 +++++++++++++++++++++++---- src/libsyntax/parse/mod.rs | 4 +- 5 files changed, 53 insertions(+), 15 deletions(-) diff --git a/src/librustc_trans/save/span_utils.rs b/src/librustc_trans/save/span_utils.rs index beec8071a72ba..a724cdc0229d2 100644 --- a/src/librustc_trans/save/span_utils.rs +++ b/src/librustc_trans/save/span_utils.rs @@ -69,8 +69,8 @@ impl<'a> SpanUtils<'a> { 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(), } } diff --git a/src/librustc_trans/trans/debuginfo.rs b/src/librustc_trans/trans/debuginfo.rs index 1e788351172fc..4d4a2bf48548a 100644 --- a/src/librustc_trans/trans/debuginfo.rs +++ b/src/librustc_trans/trans/debuginfo.rs @@ -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"}" { diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index 248ce99ff9b72..611251d4cfae1 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -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 diff --git a/src/libsyntax/codemap.rs b/src/libsyntax/codemap.rs index 00857d10f439e..3231342cb50c8 100644 --- a/src/libsyntax/codemap.rs +++ b/src/libsyntax/codemap.rs @@ -437,18 +437,35 @@ impl CodeMap { FileLines {file: lo.file, lines: lines} } - pub fn span_to_snippet(&self, sp: Span) -> Option { + pub fn span_to_snippet(&self, sp: Span) -> Result { + 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 @@ impl CodeMap { } } +#[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 @@ mod test { 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] diff --git a/src/libsyntax/parse/mod.rs b/src/libsyntax/parse/mod.rs index 6ff5c77f5079a..9e9ec08da038e 100644 --- a/src/libsyntax/parse/mod.rs +++ b/src/libsyntax/parse/mod.rs @@ -1233,8 +1233,8 @@ mod test { 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"), } } }