-
Notifications
You must be signed in to change notification settings - Fork 88
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
Stop using runtime C++ exceptions #145
base: master
Are you sure you want to change the base?
Conversation
As of LLVM 18, llvm-config contains -fno-exceptions, general standard are also moving off runtime exceptions in C++ https://stackoverflow.com/questions/4506369/when-and-how-should-i-use-exception-handling .
@@ -204,7 +204,7 @@ class Tokenizer { | |||
<< ":" << column << "\n\n"; | |||
|
|||
std::cerr << errMsg.str(); | |||
throw new std::runtime_error(errMsg.str().c_str()); | |||
std::exit(1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be EXIT_FAILURE as well?
@maelhos, thanks for the contribution! Hopefully this is not a breaking change anywhere (except the cases which may catch for the specific exceptions, but that should be fine). Can you confirm the |
For that you would need stdlib.h, it doesn't seem to be included in this part of the template though it probably is in the final header, I used 1 to play it safe but if you have the ability to test if EXIT_FAILURE works that would be great. |
You can add it here: https://github.com/DmitrySoshnikov/syntax/blob/master/src/plugins/cpp/templates/lr.template.h#L23 Where you able to test the generated code? I'll try looking at #141 later if you're blocked on testing, unless you reach that issue earlier. |
I dont't think it is possible to test generation without solving #141 since it happens at compile time though the package syntax-cli available through npm still works fine. |
Yes, good idea - we can probably change it in the follow up PR. For this one - can you update please to add |
Actually, a program should be able to handle parse error at runtime. With the full exit this now changes semantics, right? - you can't catch the parse errors anymore? |
Replace
throw
bystd::exit
As of LLVM 18, llvm-config --cxxflags contains -fno-exceptions, general standards are also moving out runtime exceptions in C++ as much as possible c.f. stackoverflow discussion.
I can't run test since the dev is broken because of transition from babel 6 to 7 (I don't know enough about babel to update everything to babel 7) and there is no version nor instruction specifying the required npm and node.