Skip to content

Commit 2e14a0e

Browse files
authored
ISLE: provide locations in errors in basic non-miette mode. (#4151)
In #4143 we made ISLE compilation part of the normal build flow again, to avoid the issues with the checked-in source. To make this acceptably fast, we cut down dependencies of the ISLE compiler, so the "fancy" error printing is now optional. When not included, it just prints error messages to stderr in a list. However, this did not include file locations. It might be nice to have this without enabling the "fancy printing" and waiting for that to build. Fortunately most of the plumbing for this was already present (we had it at one point before switching to miette). This PR adds back locations to the basic error output. It now looks like: ``` Error building ISLE files: ISLE errors: src/isa/aarch64/inst.isle:1:1: parse error: Unexpected token Symbol("asdf") ```
1 parent c4eab2b commit 2e14a0e

File tree

5 files changed

+49
-10
lines changed

5 files changed

+49
-10
lines changed

cranelift/isle/isle/src/error.rs

Lines changed: 39 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22
33
use std::sync::Arc;
44

5+
use crate::lexer::Pos;
6+
57
/// Either `Ok(T)` or `Err(isle::Error)`.
68
pub type Result<T> = std::result::Result<T, Error>;
79

@@ -82,8 +84,30 @@ impl std::fmt::Display for Error {
8284
fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
8385
match self {
8486
Error::IoError { context, .. } => write!(f, "{}", context),
87+
88+
// Include locations directly in the `Display` output when
89+
// we're not wrapping errors with miette (which provides
90+
// its own way of showing locations and context).
91+
#[cfg(not(feature = "miette-errors"))]
92+
Error::ParseError { src, span, msg, .. } => write!(
93+
f,
94+
"{}: parse error: {}",
95+
span.from.pretty_print_with_filename(&*src.name),
96+
msg
97+
),
98+
#[cfg(not(feature = "miette-errors"))]
99+
Error::TypeError { src, span, msg, .. } => write!(
100+
f,
101+
"{}: type error: {}",
102+
span.from.pretty_print_with_filename(&*src.name),
103+
msg
104+
),
105+
106+
#[cfg(feature = "miette-errors")]
85107
Error::ParseError { msg, .. } => write!(f, "parse error: {}", msg),
108+
#[cfg(feature = "miette-errors")]
86109
Error::TypeError { msg, .. } => write!(f, "type error: {}", msg),
110+
87111
Error::Errors(_) => write!(
88112
f,
89113
"found {} errors:\n\n{}",
@@ -143,17 +167,27 @@ impl Source {
143167
#[derive(Clone, Debug)]
144168
pub struct Span {
145169
/// The byte offset of the start of the span.
146-
pub from: usize,
170+
pub from: Pos,
147171
/// The byte offset of the end of the span.
148-
pub to: usize,
172+
pub to: Pos,
149173
}
150174

151175
impl Span {
152176
/// Create a new span that covers one character at the given offset.
153-
pub fn new_single(offset: usize) -> Span {
177+
pub fn new_single(pos: Pos) -> Span {
154178
Span {
155-
from: offset,
156-
to: offset + 1,
179+
from: pos,
180+
// This is a slight hack (we don't actually look at the
181+
// file to find line/col of next char); but the span
182+
// aspect, vs. just the starting point, is only relevant
183+
// for miette and when miette is enabled we use only the
184+
// `offset` here to provide its SourceSpans.
185+
to: Pos {
186+
file: pos.file,
187+
offset: pos.offset + 1,
188+
line: pos.line,
189+
col: pos.col + 1,
190+
},
157191
}
158192
}
159193
}

cranelift/isle/isle/src/error_miette.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use miette::{SourceCode, SourceSpan};
88

99
impl From<Span> for SourceSpan {
1010
fn from(span: Span) -> Self {
11-
SourceSpan::new(span.from.into(), span.to.into())
11+
SourceSpan::new(span.from.offset.into(), span.to.offset.into())
1212
}
1313
}
1414

cranelift/isle/isle/src/lexer.rs

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,12 +45,17 @@ pub struct Pos {
4545
impl Pos {
4646
/// Print this source position as `file.isle:12:34`.
4747
pub fn pretty_print(&self, filenames: &[Arc<str>]) -> String {
48-
format!("{}:{}:{}", filenames[self.file], self.line, self.col)
48+
self.pretty_print_with_filename(&filenames[self.file])
4949
}
5050
/// Print this source position as `file.isle line 12`.
5151
pub fn pretty_print_line(&self, filenames: &[Arc<str>]) -> String {
5252
format!("{} line {}", filenames[self.file], self.line)
5353
}
54+
/// As above for `pretty_print`, but with the specific filename
55+
/// already provided.
56+
pub fn pretty_print_with_filename(&self, filename: &str) -> String {
57+
format!("{}:{}:{}", filename, self.line, self.col)
58+
}
5459
}
5560

5661
/// A token of ISLE source.
@@ -167,7 +172,7 @@ impl<'a> Lexer<'a> {
167172
self.filenames[pos.file].clone(),
168173
self.file_texts[pos.file].clone(),
169174
),
170-
span: Span::new_single(self.pos().offset),
175+
span: Span::new_single(self.pos()),
171176
}
172177
}
173178

cranelift/isle/isle/src/parser.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ impl<'a> Parser<'a> {
3939
self.lexer.filenames[pos.file].clone(),
4040
self.lexer.file_texts[pos.file].clone(),
4141
),
42-
span: Span::new_single(pos.offset),
42+
span: Span::new_single(pos),
4343
}
4444
}
4545

cranelift/isle/isle/src/sema.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -727,7 +727,7 @@ impl TypeEnv {
727727
self.filenames[pos.file].clone(),
728728
self.file_texts[pos.file].clone(),
729729
),
730-
span: Span::new_single(pos.offset),
730+
span: Span::new_single(pos),
731731
};
732732
log!("{}", e);
733733
e

0 commit comments

Comments
 (0)