Skip to content
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

Add missing errno constants #8303

Closed
wants to merge 3 commits into from

Conversation

j8r
Copy link
Contributor

@j8r j8r commented Oct 10, 2019

That's mostly a POC to improve the error handling around Errno, as pointed by @bararchy on Gitter.

Content of this PR

  • Reordering the LibC Errno constants to facilitate their read.
  • Add missing Errno constants (which also make the language compiling for the next commit).
  • Add the Error::Errno exception class.

Goal

Having different error class for each Errno allows to rescue them, instead of having to compare their integer values with ==.
The end user would also be able to figure out what's the error name instead of having the Errno class with #errno as an integer.

What's next

There are still places where the Errno::Error is exposed directly instead of a subclass.

This is an important breaking change, we have to weigh carefully the pros/cons before any action.

@j8r j8r force-pushed the add-missing-errno-constants branch 2 times, most recently from 7150dd7 to b142dd7 Compare October 10, 2019 17:17
@j8r j8r force-pushed the add-missing-errno-constants branch from b142dd7 to 3dd452a Compare October 10, 2019 17:21
@ysbaddaden
Copy link
Contributor

Having different classes for Errno was rejected years ago. It creates lots of classes and increased compilation by a non negligible factor.

All I see here is noise (don't mess with libc please) and confusion around Errno::Error vs the Errno Enum for the time being.

@straight-shoota
Copy link
Member

This topic should be discussed in an issue, not a PR.

And we're certainly not going to map each errno value to a separate error type.
But we could replace Errno with a number of more specific error classes, which would essentially group related errnos into one type. This has been proposed before, but never really discussed. Related discussions are in #1124 and #4835. Please open an issue for this.

@ysbaddaden
Copy link
Contributor

ysbaddaden commented Oct 10, 2019

See #445 (quite a small number) where that was implemented and rejected.

@straight-shoota
Copy link
Member

@ysbaddaden #445 was duplicating each errno constant as a distinct type (same as this PR). That's not going to work, yes. But an alternative approach as described in #1124 (comment) would be worth looking at.

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.

None yet

3 participants