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

[Feature Request]: mark BitException::getErrorCode() as const #63

Closed
1 task done
DymOK93 opened this issue Jan 1, 2022 · 3 comments · Fixed by #64
Closed
1 task done

[Feature Request]: mark BitException::getErrorCode() as const #63

DymOK93 opened this issue Jan 1, 2022 · 3 comments · Fixed by #64
Assignees
Labels

Comments

@DymOK93
Copy link
Contributor

DymOK93 commented Jan 1, 2022

Feature description

Hi!
BitException::getErrorCode() doesn't change the internal state of the exception object and therefore should be const.
This will allow accepting an exception by a constant reference in the catch block:

try {
   ...
} catch (const BitException& exc) {
  std::cerr << "Unable to unpack an archive: error " << std::hex << exc.getErrorCode() << std::endl;
}

Additional context

No response

Code of Conduct

@DymOK93
Copy link
Contributor Author

DymOK93 commented Jan 1, 2022

Done in #64

@DymOK93
Copy link
Contributor Author

DymOK93 commented Jan 1, 2022

Oh, I see you have completely reworked the BitException in the develop branch, so take into account my remark about the noexcept specifier, please. You need to use a macro BOOST_NOEXCEPT from the Boost library or implement it yourself.

@rikyoz
Copy link
Owner

rikyoz commented Jan 2, 2022

Hi! First of all, thanks for the issue and the pull request!

BitException::getErrorCode() doesn't change the internal state of the exception object and therefore should be const.
This will allow accepting an exception by a constant reference in the catch block:

Yeah, this was an oversight by me. It should have been const from the beginning!

Done in #64

I actually already fixed this in c9c5204, but then forgot to backport the change to the master branch. I will merge your pull request so that bit7z v3 gets the fix as well. And I will probably have to release a new patched version, 3.1.5.

Oh, I see you have completely reworked the BitException in the develop branch

Yes, mainly to support a cross-platform use of the library. Now BitException extends std::system_error, instead of std::runtime_error.

so take into account my remark about the noexcept specifier, please. You need to use a macro BOOST_NOEXCEPT from the Boost library or implement it yourself.

Thanks for pointing that out! Yeah, the next version of bit7z (4.0) will actually drop the support for MSVC2012. It will require a compiler supporting at least C++14, so essentially from MSVC2015 or later. So, there will be no problem with noexcept.
I decided to stick to newer MSVC compilers to clean up the code, and MSVC2012's issues with noexcept also contributed to this decision.

Thank you again! 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants