Skip to content

[Diagnostics] Add new groups to the 'deprecated' diagnostic group #76454

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Sep 30, 2024
Merged

[Diagnostics] Add new groups to the 'deprecated' diagnostic group #76454

merged 1 commit into from
Sep 30, 2024

Conversation

DmT021
Copy link
Contributor

@DmT021 DmT021 commented Sep 13, 2024 •
edited
Loading

https://forums.swift.org/t/diagnostic-groups-deprecated/74581

This change
Removes:

  • deprecated (see the discussion)

Renames:

  • availability_deprecated to DeprecatedDeclaration (conformance_deprecated and witness_deprecated and "deprecated protocol conformance" are in this group as well)
  • unknown_warning_group to UnknownWarningGroup

@DmT021 DmT021 requested a review from a team as a code owner September 23, 2024 20:22
DmT021
DmT021 commented Sep 23, 2024
View reviewed changes
test/diagnostics/warnings_as_errors_rules.swift
@@ -1,10 +1,7 @@
// RUN: not %target-swift-frontend -typecheck -diagnostic-style llvm -warnings-as-errors %s 2>&1 | %FileCheck %s --check-prefix=CHECK-WAE
// RUN: not %target-swift-frontend -typecheck -diagnostic-style llvm -Werror availability_deprecated %s 2>&1 | %FileCheck %s --check-prefix=CHECK-WE-GROUP
// RUN: not %target-swift-frontend -typecheck -diagnostic-style llvm -Werror deprecated %s 2>&1 | %FileCheck %s --check-prefix=CHECK-WE-SUPERGROUP
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm removing -SUPERGROUP tests here because we don't currently have any. I'll bring it back in a future PR (probably with the Concurrency supergroup).

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, that's fine.

@DmT021 DmT021 force-pushed the wp/deprecated-diagnostic-groups branch from e02d7e1 to aa5e10f Compare September 25, 2024 21:18
@DmT021
Copy link
Contributor Author

DmT021 commented Sep 25, 2024

@DougGregor Can you take a look and run tests, please?

@AnthonyLatsis
Copy link
Collaborator

@swift-ci please smoke test Linux

@DmT021
Copy link
Contributor Author

DmT021 commented Sep 26, 2024

@AnthonyLatsis Can you run the other tests as well?

@DougGregor
Copy link
Member

@swift-ci please smoke test Windows

@DougGregor
Copy link
Member

@swift-ci please smoke test macOS

DougGregor
DougGregor approved these changes Sep 26, 2024
View reviewed changes
Copy link
Member

@DougGregor DougGregor left a comment

Choose a reason for hiding this comment

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

This looks great. I might quibble with the removal of the Deprecated supergroup, but it shouldn't prevent this PR from landing.

test/diagnostics/warnings_as_errors_rules.swift
@@ -1,10 +1,7 @@
// RUN: not %target-swift-frontend -typecheck -diagnostic-style llvm -warnings-as-errors %s 2>&1 | %FileCheck %s --check-prefix=CHECK-WAE
// RUN: not %target-swift-frontend -typecheck -diagnostic-style llvm -Werror availability_deprecated %s 2>&1 | %FileCheck %s --check-prefix=CHECK-WE-GROUP
// RUN: not %target-swift-frontend -typecheck -diagnostic-style llvm -Werror deprecated %s 2>&1 | %FileCheck %s --check-prefix=CHECK-WE-SUPERGROUP
Copy link
Member

Choose a reason for hiding this comment

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

Yeah, that's fine.

@DmT021
Copy link
Contributor Author

DmT021 commented Sep 26, 2024

This looks great. I might quibble with the removal of the Deprecated supergroup, but it shouldn't prevent this PR from landing.

Yeah, I'm not sure about the removal as well but some folks have raised a discussion about whether we should group the language-level deprecations with sources-level deprecations. I feel like it wouldn't be a problem but this discussion may take a while, and I want to land the minimal production-ready support for the feature before 6.1 is branched (so we have at least something usable).
The discussion is here

@DougGregor
Copy link
Member

I feel like it wouldn't be a problem but this discussion may take a while, and I want to land the minimal production-ready support for the feature before 6.1 is branched (so we have at least something usable).

I am 100% on board with this approach.

@DmT021
Copy link
Contributor Author

DmT021 commented Sep 30, 2024

@DougGregor Can you merge this, please?

@DougGregor DougGregor merged commit cd6864a into swiftlang:main Sep 30, 2024
3 checks passed
@DougGregor
Copy link
Member

@DougGregor Can you merge this, please?

Yes, done!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DougGregor DougGregor DougGregor approved these changes

@hborla hborla Awaiting requested review from hborla hborla is a code owner

@slavapestov slavapestov Awaiting requested review from slavapestov slavapestov is a code owner

@xedin xedin Awaiting requested review from xedin xedin is a code owner

@AnthonyLatsis AnthonyLatsis Awaiting requested review from AnthonyLatsis AnthonyLatsis is a code owner

@xymus xymus Awaiting requested review from xymus

@ahoppen ahoppen Awaiting requested review from ahoppen

@bnbarham bnbarham Awaiting requested review from bnbarham

@CodaFi CodaFi Awaiting requested review from CodaFi

@hamishknight hamishknight Awaiting requested review from hamishknight

@rintaro rintaro Awaiting requested review from rintaro

@ktoso ktoso Awaiting requested review from ktoso ktoso is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@DmT021 @AnthonyLatsis @DougGregor