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

Are deprecation warnings disabled? #6106

Closed
gassmoeller opened this issue Oct 23, 2024 · 3 comments · Fixed by #6115
Closed

Are deprecation warnings disabled? #6106

gassmoeller opened this issue Oct 23, 2024 · 3 comments · Fixed by #6115

Comments

@gassmoeller
Copy link
Member

gassmoeller commented Oct 23, 2024

This is somewhat related to #6101, but while working on #6097 I noticed that for a while now we compile with -Wno-deprecated-declarations (inherited from deal.II), which for me disables all deprecation warnings. This is a bit worrying, because we never see if external plugins (or ASPECT itself) use deprecated functionality of ASPECT. We should at the very least make sure our CI throws more errors and deprecation warnings, e.g. using -DASPECT_ADDITIONAL_CXX_FLAGS='-Wdeprecated-declarations -Wdeprecated-enum-enum-conversion in the CI tester, but I think we should probably enable deprecation warnings for ASPECT itself and every project using it (also to see if we use deprecated deal.II functionality). I will see if I can in a separate PR address all the deprecation warnings that we already have in the code base.

@bangerth
Copy link
Contributor

Do you know why deal.II uses these flags? I get why we use when compiling deal.II itself, but we shouldn't be using these flags when compiling user codes.

@tamiko ?

@gassmoeller
Copy link
Member Author

I tried to do some digging in the history. I think the most relevant change was this PR https://github.com/dealii/dealii/pull/14491/files#diff-afb543fb58675e305cc4ad2a8510f10c024620b036bec9f0dbe877d77ac2b5f9L72. It looks like deprecation warnings used to be disabled inside deal.II by default, but were enabled by removing the flag for dependent projects like ASPECT. To me it looks like dealii/dealii#14491 removed this step, keeping them disabled for dependent projects. I can open an issue in deal.II and we can discuss whether to enable the warnings again.

@tjhei
Copy link
Member

tjhei commented Oct 28, 2024

Yes, this seems to be unintentional. Can you open an issue?

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 a pull request may close this issue.

3 participants