[swift-evolution] Pitch: Omit deprecation warnings for same-file references
Zach Waldowski
zach at waldowski.me
Sat May 6 09:26:25 CDT 2017
I understand the reasoning and am in favor of it for those reasons, but
I will note that I use deprecations extremely frequently while
refactoring, and this would defeat that use case. As I have a few larger
projects with long compile times, losing that functionality would be
disappointing.
Sincerely,
Zachary Waldowski
zach at waldowski.me
On Fri, May 5, 2017, at 02:12 PM, Tony Allevato via swift-evolution wrote:> Hey Swift Evolvers,
>
> I'd like to propose a change that would suppress deprecation warnings
> when the reference to the deprecated declaration is in the same file
> as that declaration.>
> This personally affects one of my projects (Swift protocol buffers)
> because .proto files allow declarations to be declared as deprecated,
> and we'd like to surface that by deprecating the equivalent
> declaration in the Swift code that we generate. We can't do this
> without generating noisy build logs, because we still need to
> reference the deprecated declarations to encode/decode the messages
> correctly; and the workarounds have significant performance penalties
> as described in the draft.>
> I'd love some feedback from folks to know whether this—or something
> like it—is something that seems desirable/undesirable for the
> language. I've tinkered with an implementation in a branch[1] and it's
> fairly straightforward.>
> Gist link:
> https://gist.github.com/allevato/76f098a761147f3be5017d10a6f4ecbb>
>
> Omit deprecation warnings for same-file references
> * Proposal: SE-0000[2]
> * Author(s): Tony Allevato[3]
> * Status: Awaiting review[4]
> * Review manager: TBD> Introduction
> Public API developers can use the @available attribute in Swift to
> mark APIs as deprecated, which will emit warnings when a user
> references that API to notify them that that particular API is no
> longer preferred or may disappear in the future.> These warnings are emitted for *any* reference to a deprecated
> entity, including those in the same file. In some cases, however, it
> may be necessary and correct to continue referring to the deprecated
> entity privately while discouraging its external use. In these
> scenarios, the warnings are superfluous and generate noise in the
> build logs. We propose eliminating these warnings for references made
> in the same file.> Swift-evolution thread: TBD[5]
> Motivation
> As APIs evolve, declarations may become deprecated but still need to
> be referenced privately in order to function correctly. For example,
> consider an object that is serialized/parsed between a client and
> server. The first iteration of such a type might look like this (the
> examples use a strawman API to avoid tying the discussion too closely
> with the new coding APIs, which is not the focus):> public class Person { public var name: String public var
> phoneNumber: String? public init(name: String, phoneNumber: String?
> = nil) { self.name = name self.phoneNumber = phoneNumber } public
> convenience init?(from reader: Reader) { guard let name =
> reader.readString(withKey: "name") else { return nil } let
> phoneNumber = reader.readString(withKey: "phoneNumber")
> self.init(name: name, phoneNumber: phoneNumber) } public func
> write(to writer: Writer) { writer.write(name, key: "name") if let
> phoneNumber = phoneNumber { writer.write(phoneNumber, key:
> "phoneNumber") } } }> Later, we decide that we need to support storing multiple phone
> numbers for a Person. To avoid breaking source compatibility, we
> deprecate the old property and add a new one. We still wish the
> encoding/decoding process to preserve the integrity of the old data,
> however—for example, a middleman process used for logging should not
> modify the data stream, so the encoding/decoding process should not be
> also migrating the data. Thus, we update the class to the following:> public class Person { public var name: String @available(*,
> deprecated, message: "use 'phoneNumbers' instead") public var
> phoneNumber: String? public var phoneNumbers: [String]
> @available(*, deprecated, message: "use 'init(name:phoneNumbers:)'
> instead") public convenience init(name: String, phoneNumber: String?)
> { self.phoneNumber = phoneNumber self.init(name: name, phoneNumbers:
> []) } public init(name: String, phoneNumbers: [String] = []) {
> self.name = name self.phoneNumber = nil self.phoneNumbers =
> phoneNumbers } public convenience init?(from reader: Reader) {
> guard let name = reader.readString(withKey: "name") else { return nil
> } let phoneNumbers = reader.readStringArray(withKey: "phoneNumbers")
> ?? [] self.phoneNumber = reader.readString(withKey: "phoneNumber")
> self.init(name: name, phoneNumbers: phoneNumbers) } public func
> write(to writer: Writer) { writer.write(name, key: "name") if let
> phoneNumber = phoneNumber { writer.write(phoneNumber, key:
> "phoneNumber") } if !phoneNumbers.isEmpty {
> writer.write(phoneNumbers, key: "phoneNumbers") } } }> This type is backwards-compatible with the previous version and will
> warn external users that they should migrate to the new version of the
> API; it will also have its values preserved exactly if a middleman
> process parses and then reserializes it.> The problem, however, is that the references to phoneNumber in
> Person's *own methods* also emit deprecation warnings. This is
> undesirable because the warnings cannot be easily avoided (see
> workarounds below). The crux of the issue is that one of the two
> statements is likely to be true:
> 1. At the time the user deprecates something, they also have the
> opportunity to migrate all references to it within the same file,
> thus eliminating the warnings there and making the file build
> cleanly.
> 2. At the time the user deprecates something, they must continue to
> reference it internally to achieve correct and performant
> behavior, thus making it currently impossible to achieve a clean
> build for the file containing that declaration.> This proposal aims to solve the second issue.
> Workarounds and their problems
> One workaround would be to shadow deprecated declarations by making
> the declaration itself private (and prepending something like an
> underscore to the name) and then using a public passthrough property
> or method. The private declaration would not be deprecated while the
> public one would be.> There are two main problems with this approach. First, it increases
> the development and maintanence cost of the deprecation because the
> new declaration has to be added and all internal references to the
> declaration must be updated.> Secondly, and more importantly, this shadowing can have significant
> performance penalties. Consider this example:> class DirectArrayHolder { @available(*, deprecated, message: "Use
> something else instead") var array: [String] = [] }
> class IndirectArrayHolder { var _array: [String] = [] @available(*,
> deprecated, message: "Use something else instead") var array:
> [String] { get { return _array } set { _array = newValue } } }
> func useDirectHolder() -> DirectArrayHolder { let holder =
> DirectArrayHolder() for _ in ...10_000 { holder.array.append("foo") }
> return holder }
> func useIndirectHolder() -> IndirectArrayHolder { let holder =
> IndirectArrayHolder() for _ in ...10_000 { holder.array.append("foo")
> } return holder }> Behaviorally, these are the same. However, the generated code is
> different because the memory management semantics change due to the
> passthrough property. With full optimizations on, useDirectHolder
> executes in 1.360 sec while useIndirectHolder executes in 235.8
> sec—over two orders of magnitude slower!> It could be argued that this performance issue is something that
> could be solved separately, but this should be considered just
> one motivating example and not the sole motivation for this
> proposed change.> Proposed solution
> Do not print deprecation warnings for references to deprecated
> declarations that are in the same file.> There would be no change for declarations marked unavailable. A
> declaration marked unavailable is making a stronger statement about
> its existence such that referencing it is an error; unlike marking a
> declaration deprecated, marking it unavailable is a breaking change
> for downstream clients, so the behavior of that attribute value should
> be unchanged whether references are in the same file or elsewhere.> Implementation details
> A small addition would be made to TypeChecker::diagnoseIfDeprecated so
> that it returns early if the declaration is in the same source file as
> the reference.> Impact on existing code
> This is not a source-breaking change. The only side effect for
> existing code is that deprecation warnings that were emitted for same-
> file references in the past will no longer be emitted.> Alternatives considered
> Omitting deprecation warnings for references in the same *module*, not
> only the same *file*, was also considered. An argument could be made
> that deprecation warnings are intended as messaging for external users
> of an API, so drawing the line at the module level would be more
> appropriate than at the file level. However, modules can be large and
> span a broad set of APIs contributed by many developers on multi-
> person teams, and they may want to use the @available attribute to
> phase out particular APIs for internal use as well. Omitting
> deprecation warnings for anything in the same module takes that useful
> tool away for those users.> Another idea that was brought up in a feature request[6] filed for
> this was to allow a "deprecated block" that would suppress all
> deprecation warnings inside it. This would also solve the problem
> motivating this proposal and is similar to the approach taken by other
> languages (such as Java's @SuppressWarnings("deprecation")
> annotation), but this approach has drawbacks as well. It would not
> distinguish between deprecation warnings in the same file or module
> vs. those in other files or modules (unless modifiers were added to
> it, which runs the risk of adding too much complexity for too little
> gain). Furthermore, it has the potential to be harmful if
> abused—developers could surround large code blocks with the
> deprecation suppressor and then lose any deprecation warnings that
> would have been emitted for APIs used inside that block.> _________________________________________________
> swift-evolution mailing list
> swift-evolution at swift.org
> https://lists.swift.org/mailman/listinfo/swift-evolution
Links:
1. https://github.com/apple/swift/compare/master...allevato:omit-deprecation-same-file
2. https://github.com/apple/swift-evolution/blob/master/proposals/NNNN-name.md
3. https://github.com/allevato
4. https://gist.github.com/allevato/76f098a761147f3be5017d10a6f4ecbb#rationale
5. http://thread.gmane.org/gmane.comp.lang.swift.evolution/
6. https://bugs.swift.org/browse/SR-3357
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.swift.org/pipermail/swift-evolution/attachments/20170506/e9aec4f3/attachment.html>
More information about the swift-evolution
mailing list