From bfaeab3f97b882a6f7a93686b17591774867998a Mon Sep 17 00:00:00 2001 From: Eric York Date: Sat, 11 Apr 2020 13:28:02 -0700 Subject: [PATCH 01/13] Don't crash when missing a type in a Diagnostic message. Print an empty string for the missing type. Resolves SR-12460 --- lib/AST/DiagnosticEngine.cpp | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/lib/AST/DiagnosticEngine.cpp b/lib/AST/DiagnosticEngine.cpp index 01edba7079ee9..981d140cf4362 100644 --- a/lib/AST/DiagnosticEngine.cpp +++ b/lib/AST/DiagnosticEngine.cpp @@ -583,11 +583,14 @@ static void formatDiagnosticArgument(StringRef Modifier, } break; } - case DiagnosticArgumentKind::TypeRepr: + case DiagnosticArgumentKind::TypeRepr: { assert(Modifier.empty() && "Improper modifier for TypeRepr argument"); - Out << FormatOpts.OpeningQuotationMark << Arg.getAsTypeRepr() - << FormatOpts.ClosingQuotationMark; + auto argTypeRepr = Arg.getAsTypeRepr(); + Out << FormatOpts.OpeningQuotationMark; + argTypeRepr != NULL ? Out << argTypeRepr : Out << ""; + Out << FormatOpts.ClosingQuotationMark; break; + } case DiagnosticArgumentKind::PatternKind: assert(Modifier.empty() && "Improper modifier for PatternKind argument"); Out << Arg.getAsPatternKind(); From ca01957f409e765fc7b34e8cd4e4a4c7d5fe2701 Mon Sep 17 00:00:00 2001 From: Eric York Date: Sat, 11 Apr 2020 23:07:47 -0700 Subject: [PATCH 02/13] Added new diagnostic message, eg. "operator '==' declared in extension must be 'static'" Added assert for missing a type name IsStaticRequest::evaluate now checks for ED->getExtendedTypeRepr(). --- include/swift/AST/DiagnosticsSema.def | 3 +++ lib/AST/DiagnosticEngine.cpp | 13 ++++++------- lib/Sema/TypeCheckDecl.cpp | 14 ++++++++++---- 3 files changed, 19 insertions(+), 11 deletions(-) diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index 6428c23eb17eb..f2f32bea0cea9 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -904,6 +904,9 @@ ERROR(nonstatic_operator_in_nominal,none, ERROR(nonstatic_operator_in_extension,none, "operator %0 declared in extension of %1 must be 'static'", (Identifier, TypeRepr*)) +ERROR(nonstatic_operator_in_extension_no_type,none, +"operator %0 declared in extension must be 'static'", +(Identifier)) ERROR(nonfinal_operator_in_class,none, "operator %0 declared in non-final class %1 must be 'final'", (Identifier, Type)) diff --git a/lib/AST/DiagnosticEngine.cpp b/lib/AST/DiagnosticEngine.cpp index 981d140cf4362..443306bc3eaba 100644 --- a/lib/AST/DiagnosticEngine.cpp +++ b/lib/AST/DiagnosticEngine.cpp @@ -583,15 +583,14 @@ static void formatDiagnosticArgument(StringRef Modifier, } break; } - case DiagnosticArgumentKind::TypeRepr: { + case DiagnosticArgumentKind::TypeRepr: assert(Modifier.empty() && "Improper modifier for TypeRepr argument"); - auto argTypeRepr = Arg.getAsTypeRepr(); - Out << FormatOpts.OpeningQuotationMark; - argTypeRepr != NULL ? Out << argTypeRepr : Out << ""; - Out << FormatOpts.ClosingQuotationMark; + assert(Arg.getAsTypeRepr() && "TypeRepr argument has no name"); + Out << FormatOpts.OpeningQuotationMark << Arg.getAsTypeRepr() + << FormatOpts.ClosingQuotationMark; break; - } - case DiagnosticArgumentKind::PatternKind: + +case DiagnosticArgumentKind::PatternKind: assert(Modifier.empty() && "Improper modifier for PatternKind argument"); Out << Arg.getAsPatternKind(); break; diff --git a/lib/Sema/TypeCheckDecl.cpp b/lib/Sema/TypeCheckDecl.cpp index 2f0f90bc9a0c1..8f7519ccb85f3 100644 --- a/lib/Sema/TypeCheckDecl.cpp +++ b/lib/Sema/TypeCheckDecl.cpp @@ -676,10 +676,16 @@ IsStaticRequest::evaluate(Evaluator &evaluator, FuncDecl *decl) const { dc->isTypeContext()) { const auto operatorName = decl->getBaseIdentifier(); if (auto ED = dyn_cast(dc->getAsDecl())) { - decl->diagnose(diag::nonstatic_operator_in_extension, - operatorName, ED->getExtendedTypeRepr()) - .fixItInsert(decl->getAttributeInsertionLoc(/*forModifier=*/true), - "static "); + if (ED->getExtendedTypeRepr() == NULL) { + decl->diagnose(diag::nonstatic_operator_in_extension_no_type, operatorName) + .fixItInsert(decl->getAttributeInsertionLoc(/*forModifier=*/true), + "static "); + } else { + decl->diagnose(diag::nonstatic_operator_in_extension, + operatorName, ED->getExtendedTypeRepr()) + .fixItInsert(decl->getAttributeInsertionLoc(/*forModifier=*/true), + "static "); + } } else { auto *NTD = cast(dc->getAsDecl()); decl->diagnose(diag::nonstatic_operator_in_nominal, operatorName, From 442068f6fead33724578753dd39c6e49471252e6 Mon Sep 17 00:00:00 2001 From: Eric York Date: Sat, 11 Apr 2020 23:11:17 -0700 Subject: [PATCH 03/13] Format update --- lib/AST/DiagnosticEngine.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/AST/DiagnosticEngine.cpp b/lib/AST/DiagnosticEngine.cpp index 443306bc3eaba..e2be56bd99849 100644 --- a/lib/AST/DiagnosticEngine.cpp +++ b/lib/AST/DiagnosticEngine.cpp @@ -590,7 +590,7 @@ static void formatDiagnosticArgument(StringRef Modifier, << FormatOpts.ClosingQuotationMark; break; -case DiagnosticArgumentKind::PatternKind: + case DiagnosticArgumentKind::PatternKind: assert(Modifier.empty() && "Improper modifier for PatternKind argument"); Out << Arg.getAsPatternKind(); break; From f2e927dc7c62e4edbe41875cf1630ea1372c1c9c Mon Sep 17 00:00:00 2001 From: Eric York Date: Sat, 11 Apr 2020 23:13:39 -0700 Subject: [PATCH 04/13] Moved if check --- lib/Sema/TypeCheckDecl.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/Sema/TypeCheckDecl.cpp b/lib/Sema/TypeCheckDecl.cpp index 8f7519ccb85f3..eb2ce0d223432 100644 --- a/lib/Sema/TypeCheckDecl.cpp +++ b/lib/Sema/TypeCheckDecl.cpp @@ -676,13 +676,13 @@ IsStaticRequest::evaluate(Evaluator &evaluator, FuncDecl *decl) const { dc->isTypeContext()) { const auto operatorName = decl->getBaseIdentifier(); if (auto ED = dyn_cast(dc->getAsDecl())) { - if (ED->getExtendedTypeRepr() == NULL) { - decl->diagnose(diag::nonstatic_operator_in_extension_no_type, operatorName) + if (ED->getExtendedTypeRepr()) { + decl->diagnose(diag::nonstatic_operator_in_extension, + operatorName, ED->getExtendedTypeRepr()) .fixItInsert(decl->getAttributeInsertionLoc(/*forModifier=*/true), "static "); } else { - decl->diagnose(diag::nonstatic_operator_in_extension, - operatorName, ED->getExtendedTypeRepr()) + decl->diagnose(diag::nonstatic_operator_in_extension_no_type, operatorName) .fixItInsert(decl->getAttributeInsertionLoc(/*forModifier=*/true), "static "); } From 01d039407e27ee8418168643880867e418562b03 Mon Sep 17 00:00:00 2001 From: Eric York Date: Sun, 12 Apr 2020 10:03:48 -0700 Subject: [PATCH 05/13] Updated alert message to "TypeRepr argument is null" Formatting updates --- lib/AST/DiagnosticEngine.cpp | 5 +++-- lib/Sema/TypeCheckDecl.cpp | 17 ++++++++--------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/lib/AST/DiagnosticEngine.cpp b/lib/AST/DiagnosticEngine.cpp index e2be56bd99849..fc80808f00a61 100644 --- a/lib/AST/DiagnosticEngine.cpp +++ b/lib/AST/DiagnosticEngine.cpp @@ -583,14 +583,15 @@ static void formatDiagnosticArgument(StringRef Modifier, } break; } + case DiagnosticArgumentKind::TypeRepr: assert(Modifier.empty() && "Improper modifier for TypeRepr argument"); - assert(Arg.getAsTypeRepr() && "TypeRepr argument has no name"); + assert(Arg.getAsTypeRepr() && "TypeRepr argument is null"); Out << FormatOpts.OpeningQuotationMark << Arg.getAsTypeRepr() << FormatOpts.ClosingQuotationMark; break; - case DiagnosticArgumentKind::PatternKind: + case DiagnosticArgumentKind::PatternKind: assert(Modifier.empty() && "Improper modifier for PatternKind argument"); Out << Arg.getAsPatternKind(); break; diff --git a/lib/Sema/TypeCheckDecl.cpp b/lib/Sema/TypeCheckDecl.cpp index eb2ce0d223432..219784adbf0a4 100644 --- a/lib/Sema/TypeCheckDecl.cpp +++ b/lib/Sema/TypeCheckDecl.cpp @@ -676,15 +676,14 @@ IsStaticRequest::evaluate(Evaluator &evaluator, FuncDecl *decl) const { dc->isTypeContext()) { const auto operatorName = decl->getBaseIdentifier(); if (auto ED = dyn_cast(dc->getAsDecl())) { - if (ED->getExtendedTypeRepr()) { - decl->diagnose(diag::nonstatic_operator_in_extension, - operatorName, ED->getExtendedTypeRepr()) - .fixItInsert(decl->getAttributeInsertionLoc(/*forModifier=*/true), - "static "); - } else { - decl->diagnose(diag::nonstatic_operator_in_extension_no_type, operatorName) - .fixItInsert(decl->getAttributeInsertionLoc(/*forModifier=*/true), - "static "); + if (ED->getExtendedTypeRepr()) { + decl->diagnose(diag::nonstatic_operator_in_extension, + operatorName, ED->getExtendedTypeRepr()) + .fixItInsert(decl->getAttributeInsertionLoc(/*forModifier=*/true), "static "); + } else { + decl->diagnose(diag::nonstatic_operator_in_extension_no_type, operatorName) + .fixItInsert(decl->getAttributeInsertionLoc(/*forModifier=*/true), + "static "); } } else { auto *NTD = cast(dc->getAsDecl()); From a6c6c09a498fe7dfe976ab1fb7ba0d80957c6725 Mon Sep 17 00:00:00 2001 From: Eric York Date: Sun, 12 Apr 2020 17:11:34 -0700 Subject: [PATCH 06/13] Moved diagnostic to one message --- include/swift/AST/DiagnosticsSema.def | 7 ++----- lib/Sema/TypeCheckDecl.cpp | 12 ++++-------- 2 files changed, 6 insertions(+), 13 deletions(-) diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index f2f32bea0cea9..c1bc125b16e84 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -902,11 +902,8 @@ ERROR(nonstatic_operator_in_nominal,none, "operator %0 declared in type %1 must be 'static'", (Identifier, DeclName)) ERROR(nonstatic_operator_in_extension,none, - "operator %0 declared in extension of %1 must be 'static'", - (Identifier, TypeRepr*)) -ERROR(nonstatic_operator_in_extension_no_type,none, -"operator %0 declared in extension must be 'static'", -(Identifier)) + "operator %0 declared in extension%select{| of %2}1 must be 'static'", + (Identifier, bool, TypeRepr*)) ERROR(nonfinal_operator_in_class,none, "operator %0 declared in non-final class %1 must be 'final'", (Identifier, Type)) diff --git a/lib/Sema/TypeCheckDecl.cpp b/lib/Sema/TypeCheckDecl.cpp index 219784adbf0a4..96f41b235c9c4 100644 --- a/lib/Sema/TypeCheckDecl.cpp +++ b/lib/Sema/TypeCheckDecl.cpp @@ -676,15 +676,11 @@ IsStaticRequest::evaluate(Evaluator &evaluator, FuncDecl *decl) const { dc->isTypeContext()) { const auto operatorName = decl->getBaseIdentifier(); if (auto ED = dyn_cast(dc->getAsDecl())) { - if (ED->getExtendedTypeRepr()) { - decl->diagnose(diag::nonstatic_operator_in_extension, - operatorName, ED->getExtendedTypeRepr()) - .fixItInsert(decl->getAttributeInsertionLoc(/*forModifier=*/true), "static "); - } else { - decl->diagnose(diag::nonstatic_operator_in_extension_no_type, operatorName) + decl->diagnose(diag::nonstatic_operator_in_extension, operatorName, + ED->getExtendedTypeRepr() != NULL, + ED->getExtendedTypeRepr()) .fixItInsert(decl->getAttributeInsertionLoc(/*forModifier=*/true), - "static "); - } + "static "); } else { auto *NTD = cast(dc->getAsDecl()); decl->diagnose(diag::nonstatic_operator_in_nominal, operatorName, From 3ed5315e2088fa619bb9c21dae6a90be063eb0b3 Mon Sep 17 00:00:00 2001 From: Eric York Date: Sun, 12 Apr 2020 17:41:39 -0700 Subject: [PATCH 07/13] Add test for extention without type --- test/decl/ext/extensions.swift | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test/decl/ext/extensions.swift b/test/decl/ext/extensions.swift index 7bfec4869eb92..5b91d14e3fdf1 100644 --- a/test/decl/ext/extensions.swift +++ b/test/decl/ext/extensions.swift @@ -38,6 +38,11 @@ func nestingTest6() { extension Foo {} // expected-error {{declaration is only valid at file scope}} } +//===--- Test extensions without type and a member that should be static. +extension { // expected-error {{expected type name in extension declaration}} + func ==(lhs: Foo, rhs: Foo) -> Bool {} // expected-error {{operator '==' declared in extension must be 'static'}} +} + //===--- Test that we only allow extensions only for nominal types. struct S1 { From 830ec434685fbc26bb4bf09716418c89eaef38b0 Mon Sep 17 00:00:00 2001 From: Eric York Date: Sun, 12 Apr 2020 20:29:08 -0700 Subject: [PATCH 08/13] Use nullptr --- lib/Sema/TypeCheckDecl.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Sema/TypeCheckDecl.cpp b/lib/Sema/TypeCheckDecl.cpp index 96f41b235c9c4..270a2ad4991e3 100644 --- a/lib/Sema/TypeCheckDecl.cpp +++ b/lib/Sema/TypeCheckDecl.cpp @@ -677,7 +677,7 @@ IsStaticRequest::evaluate(Evaluator &evaluator, FuncDecl *decl) const { const auto operatorName = decl->getBaseIdentifier(); if (auto ED = dyn_cast(dc->getAsDecl())) { decl->diagnose(diag::nonstatic_operator_in_extension, operatorName, - ED->getExtendedTypeRepr() != NULL, + ED->getExtendedTypeRepr() != nullptr, ED->getExtendedTypeRepr()) .fixItInsert(decl->getAttributeInsertionLoc(/*forModifier=*/true), "static "); From 623e08f79fe4b5c48d3d6ff9038841ae42082899 Mon Sep 17 00:00:00 2001 From: Eric York Date: Sun, 12 Apr 2020 21:26:33 -0700 Subject: [PATCH 09/13] Update test comment. --- test/decl/ext/extensions.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/decl/ext/extensions.swift b/test/decl/ext/extensions.swift index 5b91d14e3fdf1..475e2a7207b97 100644 --- a/test/decl/ext/extensions.swift +++ b/test/decl/ext/extensions.swift @@ -38,7 +38,7 @@ func nestingTest6() { extension Foo {} // expected-error {{declaration is only valid at file scope}} } -//===--- Test extensions without type and a member that should be static. +//===--- Test that we don't crash when validating members inside an extension with no type name. extension { // expected-error {{expected type name in extension declaration}} func ==(lhs: Foo, rhs: Foo) -> Bool {} // expected-error {{operator '==' declared in extension must be 'static'}} } From f3ce4d6f630ce5dc2755f74935e03d2e5c8c1b71 Mon Sep 17 00:00:00 2001 From: Eric York Date: Mon, 13 Apr 2020 19:14:41 -0700 Subject: [PATCH 10/13] Fixed test to add target-swift-ide-test -print-indexed-symbols --- test/decl/ext/extensions.swift | 5 ----- test/decl/ext/extensions_no_type.swift | 8 ++++++++ 2 files changed, 8 insertions(+), 5 deletions(-) create mode 100644 test/decl/ext/extensions_no_type.swift diff --git a/test/decl/ext/extensions.swift b/test/decl/ext/extensions.swift index 475e2a7207b97..7bfec4869eb92 100644 --- a/test/decl/ext/extensions.swift +++ b/test/decl/ext/extensions.swift @@ -38,11 +38,6 @@ func nestingTest6() { extension Foo {} // expected-error {{declaration is only valid at file scope}} } -//===--- Test that we don't crash when validating members inside an extension with no type name. -extension { // expected-error {{expected type name in extension declaration}} - func ==(lhs: Foo, rhs: Foo) -> Bool {} // expected-error {{operator '==' declared in extension must be 'static'}} -} - //===--- Test that we only allow extensions only for nominal types. struct S1 { diff --git a/test/decl/ext/extensions_no_type.swift b/test/decl/ext/extensions_no_type.swift new file mode 100644 index 0000000000000..949fd851f7ba6 --- /dev/null +++ b/test/decl/ext/extensions_no_type.swift @@ -0,0 +1,8 @@ +// RUN: %target-swift-ide-test -print-indexed-symbols -source-filename %s + +struct Foo { } + +//===--- Test that we don't crash when validating members inside an extension with no type name. +extension { // expected-error {{expected type name in extension declaration}} + static func ==(lhs: Foo, rhs: Foo) -> Bool {} // expected-error {{operator '==' declared in extension must be 'static'}} +} From eb45bb0089e6b1ec2ac5bd307d2768ad1ba783ab Mon Sep 17 00:00:00 2001 From: Eric York Date: Mon, 13 Apr 2020 20:29:41 -0700 Subject: [PATCH 11/13] Updated test file name --- test/decl/ext/extensions_no_type.swift => SR-12460.swift | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename test/decl/ext/extensions_no_type.swift => SR-12460.swift (100%) diff --git a/test/decl/ext/extensions_no_type.swift b/SR-12460.swift similarity index 100% rename from test/decl/ext/extensions_no_type.swift rename to SR-12460.swift From 99af14ae0649e1481fdc2cbb3080aded17ad1676 Mon Sep 17 00:00:00 2001 From: Eric York Date: Mon, 13 Apr 2020 20:33:24 -0700 Subject: [PATCH 12/13] Updated test file name --- SR-12460.swift => test/decl/ext/SR-12460.swift | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename SR-12460.swift => test/decl/ext/SR-12460.swift (100%) diff --git a/SR-12460.swift b/test/decl/ext/SR-12460.swift similarity index 100% rename from SR-12460.swift rename to test/decl/ext/SR-12460.swift From 06a0650e50ed64c7aeeb9620aa73166e5ede26c4 Mon Sep 17 00:00:00 2001 From: Eric York Date: Tue, 14 Apr 2020 14:19:16 -0700 Subject: [PATCH 13/13] Update test file --- test/decl/ext/SR-12460.swift | 8 -------- test/decl/ext/sr_12460.swift | 9 +++++++++ 2 files changed, 9 insertions(+), 8 deletions(-) delete mode 100644 test/decl/ext/SR-12460.swift create mode 100644 test/decl/ext/sr_12460.swift diff --git a/test/decl/ext/SR-12460.swift b/test/decl/ext/SR-12460.swift deleted file mode 100644 index 949fd851f7ba6..0000000000000 --- a/test/decl/ext/SR-12460.swift +++ /dev/null @@ -1,8 +0,0 @@ -// RUN: %target-swift-ide-test -print-indexed-symbols -source-filename %s - -struct Foo { } - -//===--- Test that we don't crash when validating members inside an extension with no type name. -extension { // expected-error {{expected type name in extension declaration}} - static func ==(lhs: Foo, rhs: Foo) -> Bool {} // expected-error {{operator '==' declared in extension must be 'static'}} -} diff --git a/test/decl/ext/sr_12460.swift b/test/decl/ext/sr_12460.swift new file mode 100644 index 0000000000000..a281d464639ee --- /dev/null +++ b/test/decl/ext/sr_12460.swift @@ -0,0 +1,9 @@ +// RUN: %target-swift-ide-test -print-indexed-symbols -source-filename %s 2>&1 | %FileCheck -check-prefix CHECK %s + +// Test that we don't crash when validating members inside an extension with no type name. + +// CHECK: :[[@LINE+1]]:11: error: expected type name in extension declaration +extension { + // CHECK: :[[@LINE+1]]:8: error: operator '==' declared in extension must be 'static' + func ==(lhs: Any, rhs: Any) -> Bool {} +}