Deprecate the CommandLine.arguments setter #69981
Conversation
Accessing CommandLine.arguments is a sendability violation, since it's global mutable state. This marks the setter as deprecated, so that we can potentially make it unavailable in Swift 6 mode.
| @swift-ci Please test |
| This starts to address #66213, which will take multiple steps due to ABI/source compatibility concerns. |
glessard left a comment
glessard left a comment There was a problem hiding this comment.
This seems like the right first step. I wouldn't mind a test that checks that the warning is emitted as expected.
| @swift-ci Please test |
| /// Usage: command <number> | ||
| public static var arguments: [String] { | ||
| get { _arguments } | ||
| @available(swift, deprecated: 5.10, message: "CommandLine.arguments will be immutable in a future version") |
There was a problem hiding this comment.
I might suggest something stronger like:
"Do not modify CommandLine.arguments. It will become read-only in a future version of Swift."
Right now there are no instructions to the developer, so it may be unclear what you're asking them to do.
There was a problem hiding this comment.
I feel like the first question someone will be asking themselves when they encounter this deprecation is "What else should I use then?!", so I think it's worth addressing that question somewhere in the docs.
There was a problem hiding this comment.
I suspect the set of developers who are mutating this property is small, but if anybody asks, "pass your own mutable copy of this array where needed" seems like simple and sound advice.
| // safely initialize the swift arguments. | ||
| public static var arguments: [String] | ||
| @usableFromInline | ||
| internal static var _arguments: [String] |
There was a problem hiding this comment.
And long-term the goal is to make this let, yeah?
| internal static var _arguments: [String] | ||
| = (0..<Int(argc)).map { String(cString: _unsafeArgv[$0]!) } | ||
| | ||
| /// An array that provides access to this program's command line arguments. |
There was a problem hiding this comment.
As far as I can see, this property is currently undocumented. Thanks for writing up something for it!
There was a problem hiding this comment.
Yeah! There were actually some minimal docs there, but they were blocked from attaching by a regular comment between them and the declaration.
| @swift-ci please test macOS |
| @swift-ci please test |
| @swift-ci please test |
| /// $ command ZZZ | ||
| /// Error: Please provide a number to square. | ||
| /// Usage: command <number> | ||
| public static var arguments: [String] { |
There was a problem hiding this comment.
I think this needs to be marked @_alwaysEmitIntoClient
There was a problem hiding this comment.
What are the ABI stability implications of doing that? Existing code will have references to arguments that were not emitted into their binaries; does that work with @_alwaysEmitIntoClient? And presumably we're OK with making _arguments into ABI if we're doing that?
There was a problem hiding this comment.
I misunderstood why the ABI test failed! It's the accessor symbols for _arguments that were added.
The typical pattern for this is to leave the ABI declaration exactly as-is and change public to @usableFromInline internal, and make the new public API @_alwaysEmitIntoClient. But I think this change here is conceptually fine because changing a stored property to a computed one is ABI compatible, and the new _arguments variable is only used on newer runtimes while old runtimes will use the stored variable accessors directly.
I think the fix is to remove @usableFromInline from _arguments. I don't think it needs to be there.
| I still think the deprecation message needs a tweak, but the implementation is good. :) |
7 participants
Accessing
CommandLine.argumentsis a sendability violation, since it's global mutable state. This marks the setter as deprecated, so that we can potentially make it unavailable in Swift 6 mode.(I also wrote some docs for
CommandLine.arguments.)