-
-
Notifications
You must be signed in to change notification settings - Fork 371
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 #2485
Comments
When trying to compile
stanc3 now says
|
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 ***@***.***> 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.
|
That's fair. Is the lazy solution of just having
OK? |
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 ***@***.***> 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.
|
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. |
On Dec 13, 2018, at 10:31 AM, Matthijs Vákár ***@***.***> 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.
|
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? |
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. |
The original version was this:
Why is this printed as '£' in the code and '\194' in the error message? Nobody's going to understand what the \194 means. |
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 ***@***.***> 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.
|
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.) |
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 ***@***.***> 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.
|
This is the current error in stanc3:
Is this fine @bob-carpenter? If not, we can transfer this to stanc3 and continue there. |
Yes, I think that's OK. Most users will figure out what that means. It would be even better to also say that an identifier was expected. |
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:
stanc gives useless error message when model source has non-ASCII characters rstan#501
feature request: unicode in source stanc3#1406
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
The text was updated successfully, but these errors were encountered: