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

Use std::error_code instead of exceptions #43

Closed
ruipacheco opened this issue Dec 21, 2019 · 3 comments
Closed

Use std::error_code instead of exceptions #43

ruipacheco opened this issue Dec 21, 2019 · 3 comments
Labels
available on master Fix is done on master branch, issue closed on next release
Milestone

Comments

@ruipacheco
Copy link

Is your feature request related to a problem? Please describe.
I'm currently trying to use this library in a codebase that should be compiled without exceptions.

Describe the solution you'd like
Replace throwing exceptions with std::error_code. Since exceptions seem to be only used in the detail:: namespace this should not break compatibility with the std:: implementation.

Describe alternatives you've considered
Write my own?

@gulrak
Copy link
Owner

gulrak commented Dec 21, 2019

I'm sorry, but it would break the standard. As an example see 'Exceptions' for std::filesystem::absolute at cppreference.com, and sure enough the implementation of ghc::filesystem::absolute without error_code throws an exception in case of an error and an implementation is required to have that version of absolute too. The idea of giving the user the choice to select a call with or without exception error handling is everywhere in std::filesystem and I would not take that away.

Error handling ist described in [fs.err.report] (30.10.7 in C++17):

Functions not having an argument of type error_code& handle errors as follows, unless otherwise specified:
— When a call by the implementation to an operating system or other underlying API results in an error that prevents the function from meeting its specifications, an exception of type filesystem_- error shall be thrown. For functions with a single path argument, that argument shall be passed to the filesystem_error constructor with a single path argument. For functions with two path arguments, the first of these arguments shall be passed to the filesystem_error constructor as the path1 argument, and the second shall be passed as the path2 argument. The filesystem_error constructor’s error_code argument is set as appropriate for the specific operating system dependent error.

So a std::filesystem implementation without exceptions would break the specification. And as stated in the readme, I am open for changes but none breaking compatibility with code expecting a filesystem implementation to throw.

So while the standard says nothing about disabling exceptions and handling disabled exceptions inside their standard library is a compiler implementation specific issue, I could go down the route to try to test if exceptions are enabled or introduce a new option to disable exceptions and and simply call std::abort() in case of an error that would lead to a throw if exceptions where enabled (what GCC does when disabling them). For the large number of compilers currently supported, implementing automatic detection would be more work than I currently could put into it for the rare use-case of having a filesystem but no exception support (e.g. __cpp_exceptions is not supported by some compilers).

If I get a clean PR that uses std::abort instead of throwing only when a new option macro is enabled (e.g. GHC_NO_EXCEPITONS) and works on the systems tested by the CI scripts, I might integrate it, but there is no chance of generally never-throwing-by-default change.

@gulrak gulrak added the wontfix This will not be worked on label Dec 21, 2019
@ruipacheco
Copy link
Author

My bad!

gulrak added a commit that referenced this issue Apr 7, 2020
fixes #60, refs #43, support for usage in projects with exceptions disabled
@gulrak
Copy link
Owner

gulrak commented Apr 8, 2020

As #61 is a PR that implemented a variant of this, current WIP version 1.3.3 (6b30995) on master and upcoming release v1.3.4 are supporting environments without exceptions.

@gulrak gulrak added available on master Fix is done on master branch, issue closed on next release and removed wontfix This will not be worked on labels Apr 8, 2020
@gulrak gulrak added this to the v1.3.4 milestone Apr 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
available on master Fix is done on master branch, issue closed on next release
Projects
None yet
Development

No branches or pull requests

2 participants