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

Modernize noexcept specifications #1191

Merged
merged 2 commits into from
Sep 8, 2023
Merged

Conversation

georgthegreat
Copy link
Contributor

@georgthegreat georgthegreat commented Aug 28, 2023

As boost::geometry mandates C++14 since 1.75, I assume this can be merged in C++03-incompatible form.

@georgthegreat georgthegreat force-pushed the noexcept branch 3 times, most recently from b37ad5d to 408fcbc Compare August 28, 2023 09:54
Copy link
Collaborator

@barendgehrels barendgehrels left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, indeed C++14 syntax is fine now

@barendgehrels
Copy link
Collaborator

One thing, @vissarion , is it better to split it into two commits?
One for library code, one for extensions?

@georgthegreat
Copy link
Contributor Author

georgthegreat commented Sep 1, 2023

I have split the changes into two commits and removed the dtors as requested

@awulkiew
Copy link
Member

awulkiew commented Sep 2, 2023

@georgthegreat Since you removed the unneded destructors would you be willing to make some other related improvements as well or would you prefer to merge this as it is?

The improvements I'm thinking about are:

  • the removal of unneeded default constructors or making them = default in case their existance is documentation-related
  • replacement of virtual in what() declaration with override

@awulkiew awulkiew added this to the 1.84 milestone Sep 2, 2023
@georgthegreat
Copy link
Contributor Author

georgthegreat commented Sep 2, 2023

@awulkiew, I am ok with doing this, but I want this PR merged without this change.
Other improvements are definitely out of scope of this PR.

@barendgehrels
Copy link
Collaborator

Thanks for splitting it.

@georgthegreat
Copy link
Contributor Author

Let's merge it.

Copy link
Member

@vissarion vissarion left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@vissarion vissarion merged commit 92c7462 into boostorg:develop Sep 8, 2023
23 checks passed
@georgthegreat georgthegreat deleted the noexcept branch September 8, 2023 10:13
@georgthegreat
Copy link
Contributor Author

@awulkiew, virtual -> override replacement is here:
https://github.com/boostorg/geometry/pull/1198/files

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

Successfully merging this pull request may close these issues.

4 participants