From b790eb5dcf2237363a1a613913b405948256c075 Mon Sep 17 00:00:00 2001 From: Ding Ye Date: Wed, 22 Aug 2018 18:13:49 +1000 Subject: [PATCH 1/3] [Parser] Improve diagnostics for special platforms in available attribute. This patch adds warnings when a version number is used on the non-specific '*' platform. In addition, it fixes some misleading warning messages on 'swift' platform. Resolves: SR-8598. --- include/swift/AST/DiagnosticsParse.def | 14 +++++++++++ lib/Parse/ParseDecl.cpp | 35 +++++++++++++++++++++++--- test/Parse/diagnose_availability.swift | 32 +++++++++++++++++++++++ 3 files changed, 77 insertions(+), 4 deletions(-) diff --git a/include/swift/AST/DiagnosticsParse.def b/include/swift/AST/DiagnosticsParse.def index c5ae646c4e39b..e6b8a7d397049 100644 --- a/include/swift/AST/DiagnosticsParse.def +++ b/include/swift/AST/DiagnosticsParse.def @@ -1328,6 +1328,20 @@ ERROR(attr_availability_expected_equal,none, ERROR(attr_availability_expected_version,none, "expected version number in '%0' attribute", (StringRef)) +WARNING(attr_availability_swift_platform_expected_option,none, + "expected '%0' option with a version number for swift platform, " + "such as 'introduced', 'deprecated', or 'obsoleted'", (StringRef)) +WARNING(attr_availability_swift_platform_deprecated,none, + "'deprecated' must have a version number for swift platform in '%0' " + "attribute", (StringRef)) +WARNING(attr_availability_swift_platform_infeasible,none, + "argument '%0' is infeasible for swift platform in '%1' attribute", + (StringRef, StringRef)) + +WARNING(attr_availability_nonspecific_platform_unexpected_version,none, + "unexpected version number for non-specific platform '*' in '%0' " + "attribute", (StringRef)) + // autoclosure ERROR(attr_autoclosure_expected_r_paren,PointsToFirstBadToken, "expected ')' in @autoclosure", ()) diff --git a/lib/Parse/ParseDecl.cpp b/lib/Parse/ParseDecl.cpp index fa75d5437235e..f5187b0e4c379 100644 --- a/lib/Parse/ParseDecl.cpp +++ b/lib/Parse/ParseDecl.cpp @@ -511,10 +511,29 @@ ParserResult Parser::parseExtendedAvailabilitySpecList( bool SomeVersion = (!Introduced.empty() || !Deprecated.empty() || !Obsoleted.empty()); - if (!PlatformKind.hasValue() && - Platform == "swift" && - SomeVersion && - PlatformAgnostic == PlatformAgnosticAvailabilityKind::None) { + if (!PlatformKind.hasValue() && Platform == "swift") { + if (PlatformAgnostic == PlatformAgnosticAvailabilityKind::Deprecated) { + diagnose(AttrLoc, + diag::attr_availability_swift_platform_deprecated, + AttrName); + return nullptr; + } + if (PlatformAgnostic == PlatformAgnosticAvailabilityKind::Unavailable) { + diagnose(AttrLoc, + diag::attr_availability_swift_platform_infeasible, + "unavailable", + AttrName); + return nullptr; + } + assert(PlatformAgnostic == PlatformAgnosticAvailabilityKind::None); + + if (!SomeVersion) { + diagnose(AttrLoc, + diag::attr_availability_swift_platform_expected_option, + AttrName); + return nullptr; + } + PlatformKind = PlatformKind::none; PlatformAgnostic = PlatformAgnosticAvailabilityKind::SwiftVersionSpecific; } @@ -528,6 +547,14 @@ ParserResult Parser::parseExtendedAvailabilitySpecList( return nullptr; } + // Warn if any version is specified for non-specific '*' platforms. + if (Platform == "*" && SomeVersion) { + diagnose(AttrLoc, + diag::attr_availability_nonspecific_platform_unexpected_version, + AttrName); + return nullptr; + } + auto Attr = new (Context) AvailableAttr(AtLoc, SourceRange(AttrLoc, Tok.getLoc()), PlatformKind.getValue(), diff --git a/test/Parse/diagnose_availability.swift b/test/Parse/diagnose_availability.swift index 506bce9f46e22..9817c005c0c29 100644 --- a/test/Parse/diagnose_availability.swift +++ b/test/Parse/diagnose_availability.swift @@ -26,3 +26,35 @@ func availableOnMultiplePlatforms() {} // expected-error@-1 {{'deprecated' can't be combined with shorthand specification 'OSX 10.0'}} // expected-error@-2 {{expected declaration}} func twoShorthandsFollowedByDeprecated() {} + + +// SR-8598: Missing/wrong warning message for '*' or 'swift' platform. + +@available(*, deprecated: 4.2) +// expected-warning@-1 {{unexpected version number for non-specific platform '*' in 'available' attribute}} +func allPlatformsDeprecatedVersion() {} + +@available(*, deprecated, obsoleted: 4.2) +// expected-warning@-1 {{unexpected version number for non-specific platform '*' in 'available' attribute}} +func allPlatformsDeprecatedAndObsoleted() {} + +@available(swift, unavailable) +// expected-warning@-1 {{argument 'unavailable' is infeasible for swift platform in 'available' attribute}} +func swiftUnavailable() {} + +@available(swift, unavailable, introduced: 4.2) +// expected-warning@-1 {{argument 'unavailable' is infeasible for swift platform in 'available' attribute}} +func swiftUnavailableIntroduced() {} + +@available(swift, deprecated) +// expected-warning@-1 {{'deprecated' must have a version number for swift platform in 'available' attribute}} +func swiftDeprecated() {} + +@available(swift, deprecated, obsoleted: 4.2) +// expected-warning@-1 {{'deprecated' must have a version number for swift platform in 'available' attribute}} +func swiftDeprecatedObsoleted() {} + +@available(swift, message: "missing valid option") +// expected-warning@-1 {{expected 'available' option with a version number for swift platform, such as 'introduced', 'deprecated', or 'obsoleted'}} +func swiftMessage() {} + From 3dca9233715283c34211f4894f488e3204782829 Mon Sep 17 00:00:00 2001 From: Ding Ye Date: Thu, 23 Aug 2018 11:18:54 +1000 Subject: [PATCH 2/3] Add a fix-it and rephrase some warning messages. --- include/swift/AST/DiagnosticsParse.def | 20 ++++----- lib/Parse/ParseDecl.cpp | 59 +++++++++++++++----------- test/Parse/diagnose_availability.swift | 18 +++++--- 3 files changed, 56 insertions(+), 41 deletions(-) diff --git a/include/swift/AST/DiagnosticsParse.def b/include/swift/AST/DiagnosticsParse.def index e6b8a7d397049..556fe66810be4 100644 --- a/include/swift/AST/DiagnosticsParse.def +++ b/include/swift/AST/DiagnosticsParse.def @@ -1328,19 +1328,19 @@ ERROR(attr_availability_expected_equal,none, ERROR(attr_availability_expected_version,none, "expected version number in '%0' attribute", (StringRef)) -WARNING(attr_availability_swift_platform_expected_option,none, - "expected '%0' option with a version number for swift platform, " - "such as 'introduced', 'deprecated', or 'obsoleted'", (StringRef)) -WARNING(attr_availability_swift_platform_deprecated,none, - "'deprecated' must have a version number for swift platform in '%0' " - "attribute", (StringRef)) -WARNING(attr_availability_swift_platform_infeasible,none, - "argument '%0' is infeasible for swift platform in '%1' attribute", +WARNING(attr_availability_swift_expected_option,none, + "expected 'introduced', 'deprecated', or 'obsoleted' in '%0' attribute " + "for platform 'swift'", (StringRef)) +WARNING(attr_availability_swift_expected_deprecated_version,none, + "expected version number with 'deprecated' in '%0' attribute for " + "platform 'swift'", (StringRef)) +WARNING(attr_availability_swift_infeasible_option,none, + "'%0' cannot be used in '%1' attribute for platform 'swift'", (StringRef, StringRef)) WARNING(attr_availability_nonspecific_platform_unexpected_version,none, - "unexpected version number for non-specific platform '*' in '%0' " - "attribute", (StringRef)) + "unexpected version number in '%0' attribute for non-specific platform " + "'*'", (StringRef)) // autoclosure ERROR(attr_autoclosure_expected_r_paren,PointsToFirstBadToken, diff --git a/lib/Parse/ParseDecl.cpp b/lib/Parse/ParseDecl.cpp index f5187b0e4c379..ff9001a9f34ee 100644 --- a/lib/Parse/ParseDecl.cpp +++ b/lib/Parse/ParseDecl.cpp @@ -315,11 +315,19 @@ ParserResult Parser::parseExtendedAvailabilitySpecList( return true; }; + struct VersionArg { + llvm::VersionTuple Version; + SourceRange Range; + SourceLoc DelimiterLoc; + bool empty() const { + return Version.empty(); + } + }; + StringRef Platform = Tok.getText(); StringRef Message, Renamed; - llvm::VersionTuple Introduced, Deprecated, Obsoleted; - SourceRange IntroducedRange, DeprecatedRange, ObsoletedRange; + VersionArg Introduced, Deprecated, Obsoleted; auto PlatformAgnostic = PlatformAgnosticAvailabilityKind::None; SyntaxParsingContext AvailabilitySpecContext( @@ -443,7 +451,9 @@ ParserResult Parser::parseExtendedAvailabilitySpecList( case IsIntroduced: case IsObsoleted: { // Items with version arguments. + SourceLoc DelimiterLoc; if (findAttrValueDelimiter()) { + DelimiterLoc = Tok.getLoc(); consumeToken(); } else { diagnose(Tok, diag::attr_availability_expected_equal, AttrName, @@ -454,24 +464,19 @@ ParserResult Parser::parseExtendedAvailabilitySpecList( break; } - auto &VersionArg = + auto &VerArg = (ArgumentKind == IsIntroduced) ? Introduced : (ArgumentKind == IsDeprecated) ? Deprecated : Obsoleted; - auto &VersionRange = (ArgumentKind == IsIntroduced) - ? IntroducedRange - : (ArgumentKind == IsDeprecated) - ? DeprecatedRange - : ObsoletedRange; - if (parseVersionTuple( - VersionArg, VersionRange, + VerArg.Version, VerArg.Range, Diagnostic(diag::attr_availability_expected_version, AttrName))) { AnyArgumentInvalid = true; if (peekToken().isAny(tok::r_paren, tok::comma)) consumeToken(); } + VerArg.DelimiterLoc = DelimiterLoc; SyntaxContext->createNodeInPlace(SyntaxKind::AvailabilityLabeledArgument); @@ -514,22 +519,19 @@ ParserResult Parser::parseExtendedAvailabilitySpecList( if (!PlatformKind.hasValue() && Platform == "swift") { if (PlatformAgnostic == PlatformAgnosticAvailabilityKind::Deprecated) { diagnose(AttrLoc, - diag::attr_availability_swift_platform_deprecated, + diag::attr_availability_swift_expected_deprecated_version, AttrName); return nullptr; } if (PlatformAgnostic == PlatformAgnosticAvailabilityKind::Unavailable) { - diagnose(AttrLoc, - diag::attr_availability_swift_platform_infeasible, - "unavailable", - AttrName); + diagnose(AttrLoc, diag::attr_availability_swift_infeasible_option, + "unavailable", AttrName); return nullptr; } assert(PlatformAgnostic == PlatformAgnosticAvailabilityKind::None); if (!SomeVersion) { - diagnose(AttrLoc, - diag::attr_availability_swift_platform_expected_option, + diagnose(AttrLoc, diag::attr_availability_swift_expected_option, AttrName); return nullptr; } @@ -547,11 +549,20 @@ ParserResult Parser::parseExtendedAvailabilitySpecList( return nullptr; } - // Warn if any version is specified for non-specific '*' platforms. + // Warn if any version is specified for non-specific platform '*'. if (Platform == "*" && SomeVersion) { - diagnose(AttrLoc, - diag::attr_availability_nonspecific_platform_unexpected_version, - AttrName); + auto diag = diagnose(AttrLoc, + diag::attr_availability_nonspecific_platform_unexpected_version, + AttrName); + if (!Introduced.empty()) + diag.fixItRemove(SourceRange(Introduced.DelimiterLoc, + Introduced.Range.End)); + if (!Deprecated.empty()) + diag.fixItRemove(SourceRange(Deprecated.DelimiterLoc, + Deprecated.Range.End)); + if (!Obsoleted.empty()) + diag.fixItRemove(SourceRange(Obsoleted.DelimiterLoc, + Obsoleted.Range.End)); return nullptr; } @@ -559,9 +570,9 @@ ParserResult Parser::parseExtendedAvailabilitySpecList( AvailableAttr(AtLoc, SourceRange(AttrLoc, Tok.getLoc()), PlatformKind.getValue(), Message, Renamed, - Introduced, IntroducedRange, - Deprecated, DeprecatedRange, - Obsoleted, ObsoletedRange, + Introduced.Version, Introduced.Range, + Deprecated.Version, Deprecated.Range, + Obsoleted.Version, Obsoleted.Range, PlatformAgnostic, /*Implicit=*/false); return makeParserResult(Attr); diff --git a/test/Parse/diagnose_availability.swift b/test/Parse/diagnose_availability.swift index 9817c005c0c29..1b35b9af90fa5 100644 --- a/test/Parse/diagnose_availability.swift +++ b/test/Parse/diagnose_availability.swift @@ -31,30 +31,34 @@ func twoShorthandsFollowedByDeprecated() {} // SR-8598: Missing/wrong warning message for '*' or 'swift' platform. @available(*, deprecated: 4.2) -// expected-warning@-1 {{unexpected version number for non-specific platform '*' in 'available' attribute}} +// expected-warning@-1 {{unexpected version number in 'available' attribute for non-specific platform '*'}} {{25-30=}} func allPlatformsDeprecatedVersion() {} @available(*, deprecated, obsoleted: 4.2) -// expected-warning@-1 {{unexpected version number for non-specific platform '*' in 'available' attribute}} +// expected-warning@-1 {{unexpected version number in 'available' attribute for non-specific platform '*'}} {{36-41=}} func allPlatformsDeprecatedAndObsoleted() {} +@available(*, introduced: 4.0, deprecated: 4.1, obsoleted: 4.2) +// expected-warning@-1 {{unexpected version number in 'available' attribute for non-specific platform '*'}} {{25-30=}} {{42-47=}} {{58-63=}} +func allPlatformsDeprecatedAndObsoleted2() {} + @available(swift, unavailable) -// expected-warning@-1 {{argument 'unavailable' is infeasible for swift platform in 'available' attribute}} +// expected-warning@-1 {{'unavailable' cannot be used in 'available' attribute for platform 'swift'}} func swiftUnavailable() {} @available(swift, unavailable, introduced: 4.2) -// expected-warning@-1 {{argument 'unavailable' is infeasible for swift platform in 'available' attribute}} +// expected-warning@-1 {{'unavailable' cannot be used in 'available' attribute for platform 'swift'}} func swiftUnavailableIntroduced() {} @available(swift, deprecated) -// expected-warning@-1 {{'deprecated' must have a version number for swift platform in 'available' attribute}} +// expected-warning@-1 {{expected version number with 'deprecated' in 'available' attribute for platform 'swift'}} func swiftDeprecated() {} @available(swift, deprecated, obsoleted: 4.2) -// expected-warning@-1 {{'deprecated' must have a version number for swift platform in 'available' attribute}} +// expected-warning@-1 {{expected version number with 'deprecated' in 'available' attribute for platform 'swift'}} func swiftDeprecatedObsoleted() {} @available(swift, message: "missing valid option") -// expected-warning@-1 {{expected 'available' option with a version number for swift platform, such as 'introduced', 'deprecated', or 'obsoleted'}} +// expected-warning@-1 {{expected 'introduced', 'deprecated', or 'obsoleted' in 'available' attribute for platform 'swift'}} func swiftMessage() {} From 033f6aca92555a93f901d5df51c4b24b1c7a2b21 Mon Sep 17 00:00:00 2001 From: Ding Ye Date: Thu, 23 Aug 2018 18:00:33 +1000 Subject: [PATCH 3/3] Fix warnings in stdlib --- stdlib/public/Platform/tgmath.swift.gyb | 2 +- stdlib/public/SDK/CoreGraphics/CGFloat.swift.gyb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/stdlib/public/Platform/tgmath.swift.gyb b/stdlib/public/Platform/tgmath.swift.gyb index 9d9484f584f5d..59095ba3facd6 100644 --- a/stdlib/public/Platform/tgmath.swift.gyb +++ b/stdlib/public/Platform/tgmath.swift.gyb @@ -326,7 +326,7 @@ public func remquo(_ x: ${T}, _ y: ${T}) -> (${T}, Int) { % if T == 'Float80': #if (arch(i386) || arch(x86_64)) && !os(Windows) % end -@available(*, deprecated: 4.2, message: +@available(swift, deprecated: 4.2, message: "use ${T}(nan: ${T}.RawSignificand) instead.") @_transparent public func nan(_ tag: String) -> ${T} { diff --git a/stdlib/public/SDK/CoreGraphics/CGFloat.swift.gyb b/stdlib/public/SDK/CoreGraphics/CGFloat.swift.gyb index 2dedc1213fd8f..978c56dfc9e60 100644 --- a/stdlib/public/SDK/CoreGraphics/CGFloat.swift.gyb +++ b/stdlib/public/SDK/CoreGraphics/CGFloat.swift.gyb @@ -575,7 +575,7 @@ public func remquo(_ x: CGFloat, _ y: CGFloat) -> (CGFloat, Int) { return (CGFloat(rem), quo) } -@available(*, deprecated: 4.2, message: +@available(swift, deprecated: 4.2, message: "use CGFloat(nan: CGFloat.RawSignificand) instead.") @_transparent public func nan(_ tag: String) -> CGFloat {