oils-for-unix / oils

Oils is our upgrade path from bash to a better language and runtime. It's also for Python and JavaScript users who avoid shell!
http://www.oilshell.org/
Other
2.85k stars 159 forks source link

arithmetic operators should be tokens with location info #429

Open andychu opened 5 years ago

andychu commented 5 years ago

complication: word.ArithId() sometimes returns Id.Word_Compound.

Then there's no token. Because arithmetic works on words.

It's used in the TdopParser as self.op_id.

jyn514 commented 5 years ago

Rust's approach to this is to use 'spans' for error messages, which have a start and end and can be composed of multiple tokens.

Span definition: https://doc.rust-lang.org/nightly/nightly-rustc/syntax_pos/struct.SpanData.html Example of an error message:

(osh) ~/.../Programming/oil ▶️ /bin/cat span.rs
fn main() {
    return 2 + 2;
}
(osh) ~/.../Programming/oil ▶️ rustc span.rs
error[E0308]: mismatched types
 --> span.rs:2:12
  |
1 | fn main() {
  |           - expected `()` because of default return type
2 |     return 2 + 2;
  |            ^^^^^ expected (), found integer

It uses offsets from the start of the file then calculates the line numbers and columns when it reports the error: https://doc.rust-lang.org/nightly/nightly-rustc/src/syntax/source_map.rs.html#328

andychu commented 5 years ago

Our strategy is similar, and I'm fairly happy with how it works (maybe except for resource usage).

https://github.com/oilshell/oil/blob/master/frontend/syntax.asdl#L69

The issue here is to save the whole token, which has a span, rather than just the Id. We do that in most places, like redirect operators, but not in the case of arithmetic operators.

That's how it generates errors like this:

$ bin/osh -c 'echo x >> /'
  echo x >> /
         ^~
[ -c flag ]:1: Can't open '/': Is a directory