Why guess as to which of these is appropriate? Couldn't you support the current and all variants of this behavior by allowing access modifiers on 'deprecated'?
* public deprecated: warning when used from a different module, behaves as though there's a public deprecated pass-through * internal deprecated: warning when used from a different file * fileprivate deprecated: warning when used from a different scope * private deprecated: synonymous with deprecated for backwards compatibility, behaves like it does today (No need for complicated parsing; SE-25 allows a higher nominal access modifier inside a lower one without warning, so it's fine to allow 'public deprecated' to decorate a private member with no effect.) On Fri, May 5, 2017 at 13:51 Tony Allevato via swift-evolution < swift-evolution@swift.org> wrote: > On Fri, May 5, 2017 at 11:37 AM BJ Homer <bjho...@gmail.com> wrote: > >> I’ve run into this problem as well, when dealing with legacy file formats >> (similar to the example in the proposal), and I agree it would be nice to >> be able to address it. We’ve got a few “permanent” warnings in our code >> base right now due to this. I’m not sure whether the deprecation should be >> disabled for the entire file, though; perhaps it should be disabled just >> within the type itself? I don’t know how that would contribute to the >> complexity of the implementation, but it seems like we would still want to >> warn about deprecated accesses between two types declared in the same file. >> > > I initially picked "same file" because (1) it appears to be dead simple to > implement and (2) the idea that if someone is marking a definition as > private in a file, they can easily go through the same file and resolve any > uses of that declaration if they no longer want them. But it's possible > that there are helper types that just happen to be in the same file where > you'd like to be warned about using those "privately allowed APIs", so > you're right that same-file suppression would make those harder to detect. > > Given that, I'd be open to either of these possibilities and I'd love to > hear what others think: > > * Give deprecation-suppression "fileprivate" semantics > * Give deprecation-suppression "private" semantics (suppressed in the same > type and in extensions of that type within the same file) > > > > >> -BJ Homer >> >> On May 5, 2017, at 12:12 PM, Tony Allevato via swift-evolution < >> swift-evolution@swift.org> 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 >> <https://github.com/apple/swift/compare/master...allevato:omit-deprecation-same-file> >> and it's fairly straightforward. >> >> Gist link: >> https://gist.github.com/allevato/76f098a761147f3be5017d10a6f4ecbb >> >> >> Omit deprecation warnings for same-file references >> >> - Proposal: SE-0000 >> >> <https://github.com/apple/swift-evolution/blob/master/proposals/NNNN-name.md> >> - Author(s): Tony Allevato <https://github.com/allevato> >> - Status: Awaiting review >> >> <https://gist.github.com/allevato/76f098a761147f3be5017d10a6f4ecbb#rationale> >> - Review manager: TBD >> >> >> <https://gist.github.com/allevato/76f098a761147f3be5017d10a6f4ecbb#introduction> >> 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 >> <http://thread.gmane.org/gmane.comp.lang.swift.evolution/> >> >> <https://gist.github.com/allevato/76f098a761147f3be5017d10a6f4ecbb#motivation> >> 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. >> >> <https://gist.github.com/allevato/76f098a761147f3be5017d10a6f4ecbb#workarounds-and-their-problems>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 0...10_000 { >> holder.array.append("foo") >> } >> return holder >> } >> func useIndirectHolder() -> IndirectArrayHolder { >> let holder = IndirectArrayHolder() >> for _ in 0...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. >> >> <https://gist.github.com/allevato/76f098a761147f3be5017d10a6f4ecbb#proposed-solution>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. >> >> <https://gist.github.com/allevato/76f098a761147f3be5017d10a6f4ecbb#implementation-details>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. >> >> <https://gist.github.com/allevato/76f098a761147f3be5017d10a6f4ecbb#impact-on-existing-code>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. >> >> <https://gist.github.com/allevato/76f098a761147f3be5017d10a6f4ecbb#alternatives-considered>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 >> <https://bugs.swift.org/browse/SR-3357> 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@swift.org >> https://lists.swift.org/mailman/listinfo/swift-evolution >> >> >> _______________________________________________ > swift-evolution mailing list > swift-evolution@swift.org > https://lists.swift.org/mailman/listinfo/swift-evolution >
_______________________________________________ swift-evolution mailing list swift-evolution@swift.org https://lists.swift.org/mailman/listinfo/swift-evolution
