Skip to content
New issue

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

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

Already on GitHub? # to your account

Prevent crash when compile fails in JIT #60

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

PearCoding
Copy link
Contributor

Simple fix to prevent thorin.opt() being called when ::compile fails due to simple syntax errors and other issues.
This is mandatory for proper safe guards, error recovery and the simple fact that syntax errors are simple to make :D

@richardmembarth
Copy link
Member

Doesn't error call std::abort, which causes program termination?

@PearCoding
Copy link
Contributor Author

Yes, thats why the new commit :P

@richardmembarth
Copy link
Member

Isn't it artic that continues after syntax errors have been detected?

@PearCoding
Copy link
Contributor Author

It does continue parsing and does not error recovery from that. artic produces a spill of unrelated syntax errors outputs, but it does not crash.

E.g., (constructed case)
let camera_dir = ERROR registry::get_global_parameter_vec3("__camera_dir", make_vec3(0,0,1));

will produce a wall of errors:

error: expected ';', got 'registry'
 in jit_531b4d2e98f8ff0e(15855, 27 - 15855, 35)
       |
 15855 |    let camera_dir  = ERROR registry::get_global_parameter_vec3("__camera_dir", make_vec3(0,0,1));
       |                          ^^^^^^^^

error: expected '}', got '::'
 in jit_531b4d2e98f8ff0e(15855, 35 - 15855, 37)
       |
 15855 |    let camera_dir  = ERROR registry::get_global_parameter_vec3("__camera_dir", make_vec3(0,0,1));
       |                                  ^^

error: expected declaration, got 'get_global_parameter_vec3'
 in jit_531b4d2e98f8ff0e(15855, 37 - 15855, 62)
       |
 15855 |    let camera_dir  = ERROR registry::get_global_parameter_vec3("__camera_dir", make_vec3(0,0,1));
       |                                    ^^^^^^^^^^^^^^^^^^^^^^^^^

error: expected declaration, got '('
 in jit_531b4d2e98f8ff0e(15855, 62 - 15855, 63)
       |
 15855 |    let camera_dir  = ERROR registry::get_global_parameter_vec3("__camera_dir", make_vec3(0,0,1));
       |                                                             ^

error: expected declaration, got '"__camera_dir"'
 in jit_531b4d2e98f8ff0e(15855, 63 - 15855, 77)
       |
 15855 |    let camera_dir  = ERROR registry::get_global_parameter_vec3("__camera_dir", make_vec3(0,0,1));
       |                                                              ^^^^^^^^^^^^^^

error: expected declaration, got ','
 in jit_531b4d2e98f8ff0e(15855, 77 - 15855, 78)
       |
 15855 |    let camera_dir  = ERROR registry::get_global_parameter_vec3("__camera_dir", make_vec3(0,0,1));
       |                                                                            ^

error: expected declaration, got 'make_vec3'
 in jit_531b4d2e98f8ff0e(15855, 79 - 15855, 88)
       |
 15855 |    let camera_dir  = ERROR registry::get_global_parameter_vec3("__camera_dir", make_vec3(0,0,1));
       |                                                                              ^^^^^^^^^

error: expected declaration, got '('
 in jit_531b4d2e98f8ff0e(15855, 88 - 15855, 89)
       |
 15855 |    let camera_dir  = ERROR registry::get_global_parameter_vec3("__camera_dir", make_vec3(0,0,1));
       |                                                                                       ^

error: expected declaration, got '0'
 in jit_531b4d2e98f8ff0e(15855, 89 - 15855, 90)
       |
 15855 |    let camera_dir  = ERROR registry::get_global_parameter_vec3("__camera_dir", make_vec3(0,0,1));
       |                                                                                        ^

error: expected declaration, got ','
 in jit_531b4d2e98f8ff0e(15855, 90 - 15855, 91)
       |
 15855 |    let camera_dir  = ERROR registry::get_global_parameter_vec3("__camera_dir", make_vec3(0,0,1));
       |                                                                                         ^

error: expected declaration, got '0'
 in jit_531b4d2e98f8ff0e(15855, 91 - 15855, 92)
       |
 15855 |    let camera_dir  = ERROR registry::get_global_parameter_vec3("__camera_dir", make_vec3(0,0,1));
       |                                                                                          ^

error: expected declaration, got ','
 in jit_531b4d2e98f8ff0e(15855, 92 - 15855, 93)
       |
 15855 |    let camera_dir  = ERROR registry::get_global_parameter_vec3("__camera_dir", make_vec3(0,0,1));
       |                                                                                           ^

error: expected declaration, got '1'
 in jit_531b4d2e98f8ff0e(15855, 93 - 15855, 94)
       |
 15855 |    let camera_dir  = ERROR registry::get_global_parameter_vec3("__camera_dir", make_vec3(0,0,1));
       |                                                                                            ^

error: expected declaration, got ')'
 in jit_531b4d2e98f8ff0e(15855, 94 - 15855, 95)
       |
 15855 |    let camera_dir  = ERROR registry::get_global_parameter_vec3("__camera_dir", make_vec3(0,0,1));
       |                                                                                             ^

error: expected declaration, got ')'
 in jit_531b4d2e98f8ff0e(15855, 95 - 15855, 96)
       |
 15855 |    let camera_dir  = ERROR registry::get_global_parameter_vec3("__camera_dir", make_vec3(0,0,1));
       |                                                                                              ^

error: expected declaration, got ';'
 in jit_531b4d2e98f8ff0e(15855, 96 - 15855, 97)
       |
 15855 |    let camera_dir  = ERROR registry::get_global_parameter_vec3("__camera_dir", make_vec3(0,0,1));

@PearCoding
Copy link
Contributor Author

I think that is an issue for another day, however :D

@richardmembarth
Copy link
Member

Yes, you can pass an option to artic to limit the number of errors.

Anyways, shouldn't the program abort when error is called instead of continuing executing (calling opt)?

@PearCoding
Copy link
Contributor Author

PearCoding commented Jan 31, 2025

It definitely should not call opt. For the standalone case it can "safely" abort, but not in the case of JIT, which might be part of a bigger application.

In some cases (Jupyter Notebooks) abort also crashes the kernel and produces no output whatsoever. The IO issue might be related to Python, but it is still annoying.

On my opinion error() should not abort at all and try to return safely, but that is another issue for another day.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants