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

Valid property access fails to compile when property has a nonmutating setter and chains into a subscript #74203

Open
stephencelis opened this issue Jun 7, 2024 · 4 comments
Assignees
Labels
availability The @available attribute and availability checking in general bug A deviation from expected or documented behavior. Also: expected but undesirable behavior.

Comments

@stephencelis
Copy link
Contributor

stephencelis commented Jun 7, 2024

Description

Adding an unavailable, nonmutating setter to a property that chains into a subscript will prevent you from compiling code that only reads through the subscript.

Reproduction

struct S {
  var p: [Int] {
    get { fatalError() }
    @available(*, unavailable)
    nonmutating set {}
  }
}

func f() {
  S().p[0]  // 🛑
}

🛑 Setter for 'p' is unavailable

This invalid diagnostic also exists for deprecations and unavailable-from-async:

struct T {
  var p1: [Int] {
    get { fatalError() }
    @available(*, deprecated)
    nonmutating set {}
  }
  var p2: [Int] {
    get { fatalError() }
    @available(*, noasync)
    nonmutating set {}
  }
}

func f() {
  T().p1[0]           // ⚠️
  Task { T().p2[0] }  // ⚠️
}

⚠️ Setter for 'p1' is deprecated

⚠️ Setter for property 'p2' is unavailable from asynchronous contexts; this is an error in Swift 6

Expected behavior

I expect no setter-specific diagnostics to emit for read-only access.

Environment

swift-driver version: 1.90.11.1 Apple Swift version 5.10 (swiftlang-5.10.0.13 clang-1500.3.9.4)
Target: arm64-apple-macosx14.0

Additional information

No response

@stephencelis stephencelis added bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. triage needed This issue needs more specific labels labels Jun 7, 2024
@emrdgrmnci
Copy link

I’ve encountered this issue in my own projects, and it’s causing significant disruptions, especially in larger codebases where properties with unavailable or deprecated setters are common.

For example, we use the following pattern frequently:

struct Example {
  var data: [String] {
    get { ["example"] }
    @available(*, unavailable)
    nonmutating set {}
  }
}

func test() {
  print(Example().data[0])  // 🛑 Error: Setter for 'data' is unavailable
}

Even though the code is read-only, we still get the error, which seems counterintuitive. It’s particularly problematic in scenarios where the property is part of a public API that should be read-only for clients, but we still want to control mutability internally.

The issue also extends to deprecations and unavailability from asynchronous contexts, as demonstrated by the initial report. This impacts our ability to progressively update our APIs without breaking existing functionality.

I strongly support addressing this bug to ensure that diagnostics are only emitted when the setter is actually invoked, not on read-only access. This would align with the expected behavior and greatly enhance developer productivity by reducing unnecessary compilation errors.

Thank you for looking into this!

@jamieQ
Copy link
Contributor

jamieQ commented Jun 15, 2024

as of #72369, this should be fixed in cases where the value from the getter is actually used. a reduction from (what i assume is) the motivating use case (here) no longer produces the erroneous warnings on the nightly toolchains.

however, there appears to still be an issue if the value isn't used, and you end up with both the misleading setter availability diagnostic in addition to the expected 'subscript is accessed but result is unused' warning.

cc @tshortli – do you happen to have any ideas for how we might accurately characterize this state such that the unavailability diagnostics aren't emitted in such cases? if that can be done, it seems like adjusting the MemberAccessContext appropriately would resolve this.

@tshortli tshortli self-assigned this Jun 15, 2024
@tshortli tshortli added availability The @available attribute and availability checking in general and removed triage needed This issue needs more specific labels labels Jun 15, 2024
@tshortli
Copy link
Contributor

Thanks for bringing it to my attention. I did make some improvements in this area and I'm glad it partially addressed this. Those improvements are in the 6.0 compiler, not just on main. The unused value case remains unsolved because it defies the type checking heuristic I implemented to detect whether the value is just an r-value and therefore won't get a write-back through the setter. I wish the logic SILGen uses to generate the code were factored in a way that they were reusable by type checking.

Out of curiosity, can someone provide a real world example of an API usage that is affected by the bug that remains for the unused value case?

@stephencelis
Copy link
Contributor Author

@tshortli I think the unused value case doesn't affect us, so I can't give you a real world example, but I'm sure there are probably uses out there :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
availability The @available attribute and availability checking in general bug A deviation from expected or documented behavior. Also: expected but undesirable behavior.
Projects
None yet
Development

No branches or pull requests

4 participants