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 warning in C++17 mode. #1191

Closed
wants to merge 9 commits into from
Closed

Conversation

bluescarni
Copy link
Contributor

@bluescarni bluescarni commented Nov 20, 2017

In C++17, static constexpr members are implicitly inline and it's deprecated to re-declare them outside the class. This fixes a GCC 7.2 -Wdeprecated warning in C++17 mode.

@wjakob
Copy link
Member

wjakob commented Nov 20, 2017

Thank you -- this looks good to me.

@jagerman
Copy link
Member

It looks good to me. I've also pushed another commit to this branch that adds -Wdeprecated to the test suite to see if it catches anything else—assuming that passes the CI builds, this should be good to merge.

@jagerman
Copy link
Member

Aha, that caught some more. Another commit pushed to address them.

@bluescarni
Copy link
Contributor Author

I got a few more methinks, but I cannot reproduce the eigen3 error on my setup.

Also rework the eigen download to extract on the fly, and to extract to
'eigen' rather than 'eigen-eigen-12345...'.
@jagerman
Copy link
Member

Perhaps it was fixed in 3.3.4; I've pushed a change to update it to see if that fixes it. (Edit: forced pushed: the first attempt was broken).

These warnings are coming from Eigen, which we can't do anything about,
so just disable the warning.
@jagerman
Copy link
Member

Okay, they are all solved now. Upgrading to Eigen 3.3.4 didn't change anything, so I just disabled -Wdeprecated in eigen.h.

jagerman pushed a commit that referenced this pull request Nov 22, 2017
This commit turns on `-Wdeprecated` in the test suite and fixes several
associated deprecation warnings that show up as a result:

- in C++17 `static constexpr` members are implicitly inline; our
  redeclaration (needed for C++11/14) is deprecated in C++17.

- various test suite classes have destructors and rely on implicit copy
  constructors, but implicit copy constructor definitions when a
  user-declared destructor is present was deprecated in C++11.

- Eigen also has various implicit copy constructors, so just disable
  `-Wdeprecated` in `eigen.h`.
@jagerman
Copy link
Member

This is now merged into master (outside of github to squash the commits properly) in ba33b2f.

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.

None yet

3 participants