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

[Clang] Remove some deprecated warnings #60428

Merged
merged 1 commit into from
Feb 6, 2025

Conversation

troopa81
Copy link
Contributor

@troopa81 troopa81 commented Feb 4, 2025

Because either

  • non working Q_NOWARN_DEPRECATED_PUSH/POP in recent clang versions
  • newly deprecated 3d classes
  • Indirect used of Qt 3d deprecated classes

@github-actions github-actions bot added this to the 3.42.0 milestone Feb 4, 2025
Copy link

github-actions bot commented Feb 4, 2025

🪟 Windows builds

Download Windows builds of this PR for testing.
Debug symbols for this build are available here.
(Built from commit 3a0a6a9)

🪟 Windows Qt6 builds

Download Windows Qt6 builds of this PR for testing.
(Built from commit 3a0a6a9)

@nyalldawson
Copy link
Collaborator

Does this hide a deprecation warning from qt? If so, I think we shouldn't do that...

@troopa81
Copy link
Contributor Author

troopa81 commented Feb 5, 2025

Does this hide a deprecation warning from qt? If so, I think we shouldn't do that...

for qgsmaskpaintengine this looks like a weird clang issue/limitation, and I fail to find a way to fix it

for 3D yes. What's the policy here ? Shall we disable warnings on deprecated, which could potentially lead to adding other deprecated call to the code waiting for someone to migrate to the new API ? or Shall we add Q_PUSH_NO_WARN macro and forget that we are using a deprecated API ?

@nyalldawson
Copy link
Collaborator

What's the policy here ?

Leave the warning in place, or replace the deprecated method. Otherwise the future qt6 port will be a pain 🤪

@troopa81
Copy link
Contributor Author

troopa81 commented Feb 5, 2025

Leave the warning in place,

OK, I removed 3d fix that would require to be fixed later

@nyalldawson
Copy link
Collaborator

@troopa81 I propose an alternative in #60470 -- I think it's preferable because it avoids adding extra complexity to the cmake files.

@troopa81
Copy link
Contributor Author

troopa81 commented Feb 6, 2025

@nyalldawson OK, I removed it from this PR to keep only the openCL thing

@rouault rouault merged commit 278eb79 into qgis:master Feb 6, 2025
30 checks passed
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.

4 participants