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 noexcept instead of dynamic exception specifications #718

Merged
merged 2 commits into from
May 30, 2020

Conversation

p12tic
Copy link
Member

@p12tic p12tic commented May 30, 2020

This PR changes code to use noexcept exclusively. Dynamic exception specifications have been removed completely out of C++20. Also macros such as _NOEXCEPT have been replaced with noexcept.

This PR depends on #717 to fix build issue due to missing #include.

@shymega
Copy link

shymega commented May 30, 2020

This has a merge conflict.. could you fix that, and push to the PR once done? Cheers.

Also, are we definitely supporting C++20 now as a minimum?

@p12tic
Copy link
Member Author

p12tic commented May 30, 2020

noexcept is from C++11, I only mentioned that we were using features that are not only deprecated, but removed from a future version of the standard.

@shymega
Copy link

shymega commented May 30, 2020

noexcept is from C++11, I only mentioned that we were using features that are not only deprecated, but removed from a future version of the standard.

Gotcha. Alright, just wanted to check really. Although, perhaps we should set a minimum C++ version... that's food for thought though. Merging now 😄

@shymega shymega merged commit 8891364 into debauchee:master May 30, 2020
@p12tic
Copy link
Member Author

p12tic commented May 30, 2020

We default to C++14 right now.

@p12tic p12tic deleted the use-noexcept branch May 30, 2020 21:03
@shymega
Copy link

shymega commented May 30, 2020

OK. Would you say that's alright for now? Of course we need to factor in portability...

@p12tic
Copy link
Member Author

p12tic commented May 30, 2020

Yes. I think we should wait until we have a good use case for a newer standard before thinking about a upgrade. At this moment I don't see any features that would be beneficial.

@shymega
Copy link

shymega commented May 30, 2020

Alright, thanks for your input; sounds like a plan!

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