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

Fix for the "-Wterminate" warnings of GCC 6 #22

Merged
merged 1 commit into from
Jul 19, 2016
Merged

Fix for the "-Wterminate" warnings of GCC 6 #22

merged 1 commit into from
Jul 19, 2016

Conversation

thrimbor
Copy link
Contributor

This is just another small thing that came up when I started using cppcodec in my C++11 project.
There were a few methods which were defined with the noexcept-specifier but included a throw-statement. In C++11, when throwing an exception inside a noexcept-method, std::terminate() gets called instead of the usual exception handling. Usually this isn't the wanted behavior, which is why GCC 6 now spits out a warning message when doing that.
I resolved this by removing the noexcept-specifier from these methods. Since they were meant to get inlined and weren't introducing variables on the stack, this shouldn't affect performance in a negative way.

@jpetso
Copy link
Collaborator

jpetso commented Jul 18, 2016

Thanks. I'll just have to test if this has any effect on the generated binary size, but it should be a workable change. In the medium-term future, I hope to replace exceptions with error codes on the lowest level as mentioned in issue #4, which would make it possible to bring back the noexcept.

@jpetso jpetso merged commit d471911 into tplgy:master Jul 19, 2016
@jpetso
Copy link
Collaborator

jpetso commented Jul 19, 2016

Checked generated binaries and differences are negligible, in my GCC 6.1.1 binary of test_cppcodec even a kilobyte or so smaller in Release and MinSizeRel compared to the baseline. (I'm not sure why this would be the case, but oh well.) No significant impact on performance either.

Thanks again, your contribution is greatly appreciated :)

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.

2 participants