-
Notifications
You must be signed in to change notification settings - Fork 6.3k
[solc] Exit code 2 for exceptions. #13633
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
Conversation
|
Sure, why not. But then I'd also use that convention in other executables like |
|
Oh, this also should have a changelog entry. |
9d0d300 to
b45fbc0
Compare
|
Also, needs a rebase (gp2 is already fixed on |
b45fbc0 to
b5e9523
Compare
|
I'll push the necessary changes here. |
b5e9523 to
07591b8
Compare
07591b8 to
5b9096a
Compare
| std::cerr << std::endl; | ||
| std::cerr << "UFO SPOTTED!" << std::endl; | ||
| throw; | ||
| std::cerr << "Uncaught exception:" << std::endl; |
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.
Any particular reason why we're not using using namespace std here, and more importantly, should we open a separate issue to address, or do it in this PR? Or do we want to just leave it as is? @cameel, I understand you've done most of the work on the phaser, so I guess it's up to you.
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.
Also, if we have two consecutive stream writes, it would make more sense to append a newline, i.e. cerr << "bla bla" << "\n" and only endl in the last write, as there's no need to flush the stream every single time. We seem to follow this pattern everywhere though, so this PR is probably not the best place to fix it.
nikola-matic
left a comment
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.
I'm good with this, but there are a couple of nits left open. @cameel please take a final look.
|
Ok, merging this... I hope it doesn't break anything anywhere, but it should be fine... |
It could be useful to distinguish normal errors from exceptions.
solcwill now return0on success,1on normal failures, but2on exceptions.See #13576 (comment).