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

janet_dobytes doesn't report line, col info for compile errors #1285

Closed
sogaiu opened this issue Sep 8, 2023 · 1 comment
Closed

janet_dobytes doesn't report line, col info for compile errors #1285

sogaiu opened this issue Sep 8, 2023 · 1 comment

Comments

@sogaiu
Copy link
Contributor

sogaiu commented Sep 8, 2023

In the context of embedding janet in TIC-80, there was a report that location information was not available in compilation error messages.

Investigation suggested that this was due to the use of janet_dobytes. Some of the relevant lines are:

                ret = janet_wrap_string(cres.error);
                if (cres.macrofiber) {
                    janet_eprintf("compile error in %s: ", sourcePath);
                    janet_stacktrace_ext(cres.macrofiber, ret, "");
                } else {
                    janet_eprintf("compile error in %s: %s\n", sourcePath,
                                  (const char *)cres.error);
                }

Note that no location information is present before the strings beginning with "compile error".

I tried patching this to look like:

                ret = janet_wrap_string(cres.error);
                int32_t line = -1, column = -1;
                // try to get error loc info from compiler, then parser if needed
                if ((cres.error_mapping.line > 0) &&
                    (cres.error_mapping.column > 0)) {
                    line = cres.error_mapping.line;
                    column = cres.error_mapping.column;
                } else if ((parser.line > 0) && (parser.column > 0)) {
                    line = parser.line;
                    column = parser.column;
                } else {
                    janet_eprintf("Sorry, no error location info found.");
                }
                if (cres.macrofiber) {
                    janet_eprintf("%d:%d: compile error in %s: ",
                                  line, column, sourcePath);
                    janet_stacktrace_ext(cres.macrofiber, ret, "");
                } else {
                    janet_eprintf("%d:%d: compile error in %s: %s\n",
                                  line, column, sourcePath,
                                  (const char *)cres.error);
                }

Basically, the changes are to check whether there is sensible-looking info available via the compilation result (cres) and if not, use location information from parsing. I don't know if the latter can fail (may be it can't?), but if it does, emit some message explaining that no location info was located.

It has worked in limited testing.

A question that came up is whether something like this [1] would be useful or sensible in janet's janet_dobytes.

Any interest?


[1] Further down in janet_dobytes, location info (including sourcePath) is provided for parse errors, so if it makes sense to modify janet_dobytes, perhaps that would be worth adding too.

@sogaiu
Copy link
Contributor Author

sogaiu commented Sep 13, 2023

Re:

[1] Further down in janet_dobytes, location info (including sourcePath) is provided for parse errors, so if it makes sense to modify janet_dobytes, perhaps that would be worth adding too.

That's not quite right.

Better to have said "make the messages for compilation errors have a format that is more in line with others", i.e. like:

<sourcePath>:<line>:<column>: <message>

The sourcePath info is in the current message, it's just currently not at the beginning:

compile error in <sourcePath>:

@sogaiu sogaiu changed the title janet_dobytes doesn't report location info for compile errors janet_dobytes doesn't report line, col info for compile errors Sep 13, 2023
@sogaiu sogaiu closed this as completed Sep 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant