Sema: Improve availability diagnostics when -target-min-inlining-version min is specified
#58963
Conversation
|
@swift-ci please test |
lib/Sema/TypeCheckAvailability.cpp
Outdated
| return Diagnostic( | ||
| IsError ? diag::availability_decl_only_version_newer_for_clients | ||
| : diag::availability_decl_only_version_newer_for_clients_warn, | ||
| D->getName(), ReferenceDC->getParentModule(), Platform, Version); |
There was a problem hiding this comment.
What is the goal of printing the reference module name here?
I'm wondering if printing it could be confusing in the case where the unavailable decl comes from a different module. You could see a View (as in SwiftUI.View) is only available to clients of MyLib in macOS 12.0 or newer, when View is actually available more widely to the clients importing SwiftUI directly. Maybe we should talk about the interface of the reference module: View is only available in the API/module interface of MyLib in macOS 12.0.
There was a problem hiding this comment.
Or should we reference D's module instead?
There was a problem hiding this comment.
The goal is to help guide the user to understand what "client" refers to in the diagnostic; I want to communicate that it's an issue for consumers of the module being built, not just any code that consumes the API (which could include other code inside the same module).
I do see that cross references could be bit confusing in general, but I didn't immediately follow the example you gave. Wouldn't the required availability of SwiftUI.View in that example be the same as its declared availability? How would SwiftUI.View be more available in general but less available in the context where the diagnostic is emitted?
There was a problem hiding this comment.
I think that using D's module name would not convey what I'm trying to convey. When this diagnostic is emitted, D would always be available to code in the module being built. The issue is that it might not be available to the clients of the module being built. The clients of D's module, however, include the module being built. So if the diagnostic were reworded to refer to D's module then it is not adding any semantic meaning over the previous diagnostic wording.
There was a problem hiding this comment.
I've update the diagnostic text to this:
'Foo' is only available in macOS X.Y or newer; clients of 'Module' may have a lower deployment target
tshortli
force-pushed
the
diagnose-potentially-unavailable-to-clients-of-module
branch
2 times, most recently
from
d2d572e to
6fe78d1
Compare
…emove the OS versions for other platforms from availability attributes. Upcoming changes to the diagnostics make it difficult to verify correct behavior when only matching a fraction of a diagnostic, so the test needs to run on a single platform in order to have stable diagnostics to match. The `availability_refinement_contexts_target_min_inlining.swift` test case runs on all ABI stable platforms and verifies that an appropriate version floor is used for each, so running this test suite on multiple platforms is somewhat redundant.
…ce when is specified.
…ve clarity for authors of API libraries. When decls are diagnosed as potentially unavailable on an OS earlier than the deployment target, the message will now indicate that the issue would be faced by clients of the module. Resolves rdar://93466875
tshortli
force-pushed
the
diagnose-potentially-unavailable-to-clients-of-module
branch
from
6fe78d1 to
a0bf64f
Compare
|
@swift-ci please smoke test |
tshortli
deleted the
diagnose-potentially-unavailable-to-clients-of-module
branch
… min` (#59176) Cherry-pick and squash of the following PRs to release/5.7: #42585 #58417 #58654 #58680 #58707 #58870 #58963 #59040 #59065 * Sema: Fix `isExported()` for extension decls in order to correct availability diagnostics when `-target-min-inlining-version min` is specified. Resolves rdar://91382040 * Sema: Avoid diagnosing potential unavailability of type components (extension nominal type, superclass, etc.) on declarations that are explicitly unavailable. Resolves rdar://92179327 * Sema: Downgrade diagnostics about potential unavailability of the extended type in an extension declaration when the following conditions are met: 1. The extension is missing explicit availability. 2. The required availability is before the deployment target. Resolves rdar://92621567 * NFC: Expand `-target-min-inlining-versiong` tests to cover a number of corner cases: - unavailable declarations and unavailable containers - SPI declarations and spi containers - property initializer expressions - property wrappers * Sema: When computing potential unavailability of a decl, first check whether the decl is explicitly unavailable and the context is also unavailable. If those conditions are met, treat the decl as if it were always available since unavailable code is allowed to reference unavailable decls. Resolves rdar://92551870 * Sema: Use the deployment target when checking availability for SPI and unavailable API declarations with `-target-min-inlining-version min` specified. There's not much benefit to more accurate enforcement of availability in these decls since API clients can't use them and there's a lot of existing code that would be needlessly diagnosed without these exceptions. Resolves rdar://92716633 * Sema: Teach the compiler to refine `VarDecl` initializer expressions using the deployment target when the init would not be exposed to module clients. Without this, the initializers of public properties in API modules could be misdiagnosed as potentially unavailable to clients of the module, even though the expression will only ever execute on the deployment target or higher. Resolves rdar://92713589 * Tests: Update `attr_inlinable_available.swift` to require macOS and remove the OS versions for other platforms from availability attributes. * Tests: Add test cases for potential unavailability in class inheritance when is specified. * Sema: Reword diagnostics about potentially unavailable decls to improve clarity for authors of API libraries. When decls are diagnosed as potentially unavailable on an OS earlier than the deployment target, the message will now indicate that the issue would be faced by clients of the module. Resolves rdar://93466875 * Sema: Use the availability of the extended nominal as a floor for the availability of extensions. The primary motivation for this change is to reduce unnecessary availability diagnostics for API library authors. Many API libraries contain existing extension decls that lack declared availability where the extension introduces additional members to the extended type in the same release that the extended type was declared. Others contain extensions where the extension itself does not have declared availability but each of the members do. In both cases, the code is safe as written so the extra diagnostics would be a nuisance. Resolves rdar://93630782 * Sema: Downgrade diagnostics about inheritance from a less available type when `-target-min-inlining-version min` is specified. As a concession to source compatibility for API libraries, downgrade diagnostics about inheritance from a less available type when the following conditions are met: 1. The inherited type is only potentially unavailable before the deployment target. 2. The inheriting type is `@usableFromInline`. Resolves rdar://92747826
Reword diagnostics about potentially unavailable decls to improve clarity for authors of API libraries. When decls are diagnosed as potentially unavailable on an OS earlier than the deployment target, the message will now indicate that the issue would be faced by clients of the module.
For example, suppose these declarations were part of an API module named
Testwith a deployment target of 10.15 on macOS. When buildingTest, the diagnostic emitted in the body of the inlinable functionfoo()now indicates that althoughbar()is available toTest, it is not necessarily available to the clients ofTest:Resolves rdar://93466875