Skip to content

Commit

Permalink
make codemap more robust in face of ill-formed spans.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
pnkfelix committed Feb 5, 2015
1 parent 189930f commit fa9d223
Show file tree
Hide file tree
Showing 5 changed files with 53 additions and 15 deletions.
4 changes: 2 additions & 2 deletions src/librustc_trans/save/span_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/librustc_trans/trans/debuginfo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"}" {
Expand Down
4 changes: 2 additions & 2 deletions src/librustdoc/clean/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
54 changes: 46 additions & 8 deletions src/libsyntax/codemap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -437,18 +437,35 @@ impl CodeMap {
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())
}
}

Expand Down Expand Up @@ -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::*;
Expand Down Expand Up @@ -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]
Expand Down
4 changes: 2 additions & 2 deletions src/libsyntax/parse/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
}
}
}

0 comments on commit fa9d223

Please sign in to comment.