Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

illegal encoding warning #188

Open
alashworth opened this issue Mar 12, 2019 · 12 comments
Open

illegal encoding warning #188

alashworth opened this issue Mar 12, 2019 · 12 comments

Comments

@alashworth
Copy link
Owner

Issue by bob-carpenter
Friday Mar 09, 2018 at 20:22 GMT
Originally opened as stan-dev/stan#2485


Summary:

We should alert users when they use non-ASCII characters outside of comments. This can be handled in the preprocessor and flagged as such.

Related issues:

Obviously if we allow UTF-8 encoded Unicode, it's a different set of illegal byte sequences we have to flag (not all byte sequences form legal UTF-8).

Current Version:

v2.17.1

@alashworth
Copy link
Owner Author

Comment by VMatthijs
Thursday Dec 13, 2018 at 12:53 GMT


When trying to compile

model {
  real £; 
}

stanc3 now says

Syntax error at file "test.stan", line 2, character 6, lexing error:
   -------------------------------------------------
     1:  model {
     2:    real £;
               ^
     3:  }
   -------------------------------------------------

Invalid input "\194"

@alashworth
Copy link
Owner Author

Comment by bob-carpenter
Thursday Dec 13, 2018 at 13:53 GMT


Is there a way to render it as you did in the program? I

I think "\194" is going to be very confusing for our users!

On Dec 13, 2018, at 7:53 AM, Matthijs Vákár [email protected] wrote:

When trying to compile

model {
real £;
}

stanc3 now says

Syntax error at file "test.stan", line 2, character 6, lexing error:

 1:  model {
 2:    real £;
           ^
 3:  }

Invalid input "\194"


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.

@alashworth
Copy link
Owner Author

Comment by VMatthijs
Thursday Dec 13, 2018 at 15:03 GMT


That's fair.

Is the lazy solution of just having

Syntax error at file "test.stan", line 2, column 6, lexing error:
   -------------------------------------------------
     1:  model {
     2:    real £;
               ^
     3:  }
   -------------------------------------------------

Invalid character found.

OK?

@alashworth
Copy link
Owner Author

Comment by bob-carpenter
Thursday Dec 13, 2018 at 15:27 GMT


I liked the first version better. It's not clear why it's illegal input with this message. Can we say a "non-ASCII character" or something like that? Or do you get the same error if you say

real 1;

or

real _abc;

neither of which is legal in Stan.

On Dec 13, 2018, at 10:03 AM, Matthijs Vákár [email protected] wrote:

That's fair.

Is the lazy solution of just having

Syntax error at file "test.stan", line 2, column 6, lexing error:

 1:  model {
 2:    real £;
           ^
 3:  }

Invalid input

OK?


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.

@alashworth
Copy link
Owner Author

Comment by VMatthijs
Thursday Dec 13, 2018 at 15:31 GMT


It's not only non-ASCII that should trigger lexer errors, right? For instance, shouldn't $ also do that? The difficulty is that ocamllex captures the problematic token and returns it to the error message, but ocamllex cannot deal with unicode properly. It might be possible to just capture the original string using the positions specified by ocamllex.

@alashworth
Copy link
Owner Author

Comment by bob-carpenter
Thursday Dec 13, 2018 at 16:10 GMT


On Dec 13, 2018, at 10:31 AM, Matthijs Vákár [email protected] wrote:

It's not only non-ASCII that should trigger lexer errors, right? For instance, shouldn't $ also do that?

That's why I asked---I didn't know how the error was being triggered.

Are there situations where we know it's an identifier, so we could say something like

expected variable name but found illegal identifier '$'


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.

@alashworth
Copy link
Owner Author

Comment by VMatthijs
Thursday Dec 13, 2018 at 16:48 GMT


We could for ASCII characters if that is preferable.

For non-ASCII, we could try replacing ocamllex with sedlex, which seems to accept unicode. Then, we could also print the £ in the error. I didn't use sedlex to start with, because I thought our conclusion was that we don't care about unicode, because C++ can't handle it. That's why I stuck with ocamllex, as it's tried and tested technology with lots of examples out there.

I can open an issue for it in stanc3.

For the time being, should I move things back to the original error?

@alashworth
Copy link
Owner Author

Comment by VMatthijs
Thursday Dec 13, 2018 at 21:52 GMT


I looked into sedlex a bit more and I think I'm tempted to stay with ocamllex for now. The switch would be some work as their syntax and wiring is quite different and I have a preference to stick with the more standard technology until more people are using sedlex or we've spoken to people who have first hand experience with it.

@alashworth
Copy link
Owner Author

Comment by bob-carpenter
Thursday Dec 13, 2018 at 23:36 GMT


The original version was this:

  -------------------------------------------------
     1:  model {
     2:    real £;
               ^
     3:  }
   -------------------------------------------------

Invalid input "\194"

Why is this printed as '£' in the code and '\194' in the error message? Nobody's going to understand what the \194 means.

@alashworth
Copy link
Owner Author

Comment by bob-carpenter
Friday Dec 14, 2018 at 00:04 GMT


That's fine, but I'd still like to know why £ renders in the code but not in the error message. Is that just a side effect of using sedlex?

Anyway, dealing with illegal character input isn't a big deal. We could always just check all the ASCII code points are under 128 and throw an error right away if they're not.

On Dec 13, 2018, at 4:52 PM, Matthijs Vákár [email protected] wrote:

I looked into sedlex a bit more and I think I'm tempted to stay with ocamllex for now. The switch would be some work as their syntax and wiring is quite different and I have a preference to stick with the more standard technology until more people are using sedlex or we've spoken to people who have first hand experience with it.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.

@alashworth
Copy link
Owner Author

Comment by VMatthijs
Friday Dec 14, 2018 at 13:49 GMT


The original version was this:

  -------------------------------------------------
     1:  model {
     2:    real £;
               ^
     3:  }
   -------------------------------------------------

Invalid input "\194"

Why is this printed as '£' in the code and '\194' in the error message? Nobody's going to understand what the \194 means.

Because OCaml can handle unicode. The snippet of code printed above is just extracted from the original file using ocamls IO, so it's fine. The '\194' in the error message is returned by ocamllex which tried to extract it from the original file, but failed as ocamllex isn't set up to work with unicode.

Could you explain once more what solution you'd like to see right now, before we do it properly? (I feel like there are more important issues at the moment. We can also improve some of these messages once the compiler is launched. At the moment, I feel like stanc3 is holding up a lot of other things, so I'd like to get it to a point where it can be swapped out ASAP.)

@alashworth
Copy link
Owner Author

Comment by bob-carpenter
Friday Dec 14, 2018 at 17:00 GMT


Thanks for the explanation on the mismatch between OCaml and the lexer on character encodings.

When the cycles are available, the best thing to do would be to precheck the input to make sure it doesn't contain non-ASCII code points outside of comments.

Another approach would be to process the escapes on the output, but that would be making strong assumptions about when <digit>+ occurs in error output from the lexer.

On Dec 14, 2018, at 8:49 AM, Matthijs Vákár [email protected] wrote:

The original version was this:


 1:  model {
 2:    real £;
           ^
 3:  }

Invalid input "\194"

Why is this printed as '£' in the code and '\194' in the error message? Nobody's going to understand what the \194 means.

Because OCaml can handle unicode. The snippet of code printed above is just extracted from the original file using ocamls IO, so it's fine. The '\194' in the error message is returned by ocamllex which tried to extract it from the original file, but failed as ocamllex isn't set up to work with unicode.

Could you explain once more what solution you'd like to see right now, before we do it properly. (I feel like there are more important issues at the moment. We can also improve some of these messages once the compiler is launched. At the moment, I feel like stanc3 is holding up a lot of other things, so I'd like to get it to a point where it can be swapped out ASAP.)


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant