Skip to content

[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

Merged
merged 1 commit into from
Nov 1, 2022
Merged

[solc] Exit code 2 for exceptions. #13633

merged 1 commit into from
Nov 1, 2022

Conversation

aarlt
Copy link
Member

@aarlt aarlt commented Oct 18, 2022

It could be useful to distinguish normal errors from exceptions. solc will now return 0 on success, 1 on normal failures, but 2 on exceptions.

See #13576 (comment).

@cameel
Copy link
Member

cameel commented Oct 19, 2022

Sure, why not. But then I'd also use that convention in other executables like isolteset or yul-phaser.

@cameel
Copy link
Member

cameel commented Oct 20, 2022

Oh, this also should have a changelog entry.

@aarlt aarlt force-pushed the solc-return-codes branch from 9d0d300 to b45fbc0 Compare October 21, 2022 22:36
@cameel
Copy link
Member

cameel commented Oct 26, 2022

Also, needs a rebase (gp2 is already fixed on develop).

@aarlt aarlt force-pushed the solc-return-codes branch from b45fbc0 to b5e9523 Compare October 26, 2022 22:16
@nikola-matic nikola-matic self-assigned this Nov 1, 2022
@nikola-matic
Copy link
Collaborator

I'll push the necessary changes here.

std::cerr << std::endl;
std::cerr << "UFO SPOTTED!" << std::endl;
throw;
std::cerr << "Uncaught exception:" << std::endl;
Copy link
Collaborator

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.

Copy link
Collaborator

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.

Copy link
Collaborator

@nikola-matic nikola-matic left a 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.

@ekpyron
Copy link
Member

ekpyron commented Nov 1, 2022

Ok, merging this... I hope it doesn't break anything anywhere, but it should be fine...

@ekpyron ekpyron merged commit 05217fa into develop Nov 1, 2022
@ekpyron ekpyron deleted the solc-return-codes branch November 1, 2022 13:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants