Skip to content

Conversation

maelhos
Copy link

@maelhos maelhos commented Jan 2, 2024

Replace throw by std::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.

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 .

std::cerr << errMsg.str();
throw new std::runtime_error(errMsg.str().c_str());
std::exit(1);
Copy link
Owner

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?

@DmitrySoshnikov
Copy link
Owner

@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 EXIT_FAILURE in the second case?

@maelhos
Copy link
Author

maelhos commented Jan 3, 2024

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.

@DmitrySoshnikov
Copy link
Owner

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.

@maelhos
Copy link
Author

maelhos commented Jan 4, 2024

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.

@DmitrySoshnikov
Copy link
Owner

#141 should be fixed 0ec6737. Can you verify?

@maelhos
Copy link
Author

maelhos commented Jan 6, 2024

#141 should be fixed 0ec6737. Can you verify?

It's fixed, everything works for me, instead of just exiting we could also make a custom Error Wrapper which could be overwritten by the user (kinda yyerror in bison) would you want that ?

@DmitrySoshnikov
Copy link
Owner

Yes, good idea - we can probably change it in the follow up PR.

For this one - can you update please to add stdlib.h in https://github.com/DmitrySoshnikov/syntax/blob/master/src/plugins/cpp/templates/lr.template.h#L23 and also change 1 to EXIT_FAILURE?

@DmitrySoshnikov
Copy link
Owner

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?

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

Successfully merging this pull request may close these issues.

2 participants