Skip to content

[Typechecker] Fix duplicate diagnostics when using attributes #28888

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Dec 20, 2019
Merged

[Typechecker] Fix duplicate diagnostics when using attributes #28888

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
49e0570
[Typechecker] resolveAttributedType() should invalidate the type repr…
theblixguy Dec 20, 2019
357d655
[Test] Update '@noescape' attribute diagnostics
theblixguy Dec 20, 2019
06b71f8
[Typechecker] Add a special diagnose method to TypeResolver that acce…
theblixguy Dec 20, 2019
3259791
[Typechecker] Rename the special 'diagnose' method to 'diagnoseInvali…
theblixguy Dec 20, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
93 changes: 55 additions & 38 deletions lib/Sema/TypeCheckType.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1778,6 +1778,14 @@ namespace {
return diags.diagnose(std::forward<ArgTypes>(Args)...);
}

template <typename... ArgTypes>
InFlightDiagnostic diagnoseInvalid(TypeRepr *repr,
ArgTypes &&... Args) const {
auto &diags = Context.Diags;
repr->setInvalid();
return diags.diagnose(std::forward<ArgTypes>(Args)...);
}

Type resolveAttributedType(AttributedTypeRepr *repr,
TypeResolutionOptions options);
Type resolveAttributedType(TypeAttributes &attrs, TypeRepr *repr,
Expand Down Expand Up @@ -2070,18 +2078,21 @@ Type TypeResolver::resolveAttributedType(TypeAttributes &attrs,

// Check for @thick.
if (attrs.has(TAK_thick)) {
if (storedRepr)
diagnose(repr->getStartLoc(), diag::sil_metatype_multiple_reprs);

if (storedRepr) {
diagnoseInvalid(repr, repr->getStartLoc(),
diag::sil_metatype_multiple_reprs);
}

storedRepr = MetatypeRepresentation::Thick;
attrs.clearAttribute(TAK_thick);
}

// Check for @objc_metatype.
if (attrs.has(TAK_objc_metatype)) {
if (storedRepr)
diagnose(repr->getStartLoc(), diag::sil_metatype_multiple_reprs);

if (storedRepr) {
diagnoseInvalid(repr, repr->getStartLoc(),
diag::sil_metatype_multiple_reprs);
}
storedRepr = MetatypeRepresentation::ObjC;
attrs.clearAttribute(TAK_objc_metatype);
}
Expand Down Expand Up @@ -2109,8 +2120,8 @@ Type TypeResolver::resolveAttributedType(TypeAttributes &attrs,

auto checkUnsupportedAttr = [&](TypeAttrKind attr) {
if (attrs.has(attr)) {
diagnose(attrs.getLoc(attr), diag::unknown_attribute,
TypeAttributes::getAttrName(attr));
diagnoseInvalid(repr, attrs.getLoc(attr), diag::unknown_attribute,
TypeAttributes::getAttrName(attr));
attrs.clearAttribute(attr);
}
};
Expand Down Expand Up @@ -2160,8 +2171,8 @@ Type TypeResolver::resolveAttributedType(TypeAttributes &attrs,
auto calleeConvention = ParameterConvention::Direct_Unowned;
if (attrs.has(TAK_callee_owned)) {
if (attrs.has(TAK_callee_guaranteed)) {
diagnose(attrs.getLoc(TAK_callee_owned),
diag::sil_function_repeat_convention, /*callee*/ 2);
diagnoseInvalid(repr, attrs.getLoc(TAK_callee_owned),
diag::sil_function_repeat_convention, /*callee*/ 2);
}
calleeConvention = ParameterConvention::Direct_Owned;
} else if (attrs.has(TAK_callee_guaranteed)) {
Expand All @@ -2187,8 +2198,9 @@ Type TypeResolver::resolveAttributedType(TypeAttributes &attrs,
SILFunctionType::Representation::WitnessMethod)
.Default(None);
if (!parsedRep) {
diagnose(attrs.getLoc(TAK_convention),
diag::unsupported_sil_convention, attrs.getConventionName());
diagnoseInvalid(repr, attrs.getLoc(TAK_convention),
diag::unsupported_sil_convention,
attrs.getConventionName());
rep = SILFunctionType::Representation::Thin;
} else {
rep = *parsedRep;
Expand All @@ -2204,8 +2216,8 @@ Type TypeResolver::resolveAttributedType(TypeAttributes &attrs,

if (attrs.has(TAK_differentiable) &&
!Context.LangOpts.EnableExperimentalDifferentiableProgramming) {
diagnose(attrs.getLoc(TAK_differentiable),
diag::experimental_differentiable_programming_disabled);
diagnoseInvalid(repr, attrs.getLoc(TAK_differentiable),
diag::experimental_differentiable_programming_disabled);
}

DifferentiabilityKind diffKind = DifferentiabilityKind::NonDifferentiable;
Expand Down Expand Up @@ -2236,8 +2248,9 @@ Type TypeResolver::resolveAttributedType(TypeAttributes &attrs,
.Case("c", FunctionType::Representation::CFunctionPointer)
.Default(None);
if (!parsedRep) {
diagnose(attrs.getLoc(TAK_convention), diag::unsupported_convention,
attrs.getConventionName());
diagnoseInvalid(repr, attrs.getLoc(TAK_convention),
diag::unsupported_convention,
attrs.getConventionName());
rep = FunctionType::Representation::Swift;
} else {
rep = *parsedRep;
Expand All @@ -2246,8 +2259,8 @@ Type TypeResolver::resolveAttributedType(TypeAttributes &attrs,

if (attrs.has(TAK_differentiable) &&
!Context.LangOpts.EnableExperimentalDifferentiableProgramming) {
diagnose(attrs.getLoc(TAK_differentiable),
diag::experimental_differentiable_programming_disabled);
diagnoseInvalid(repr, attrs.getLoc(TAK_differentiable),
diag::experimental_differentiable_programming_disabled);
}

DifferentiabilityKind diffKind = DifferentiabilityKind::NonDifferentiable;
Expand All @@ -2269,21 +2282,21 @@ Type TypeResolver::resolveAttributedType(TypeAttributes &attrs,
if (attrs.hasConvention()) {
if (attrs.getConventionName() == "c" ||
attrs.getConventionName() == "block") {
diagnose(attrs.getLoc(TAK_convention),
diag::invalid_autoclosure_and_convention_attributes,
attrs.getConventionName());
diagnoseInvalid(repr, attrs.getLoc(TAK_convention),
diag::invalid_autoclosure_and_convention_attributes,
attrs.getConventionName());
attrs.clearAttribute(TAK_convention);
didDiagnose = true;
}
} else if (options.is(TypeResolverContext::VariadicFunctionInput) &&
!options.hasBase(TypeResolverContext::EnumElementDecl)) {
diagnose(attrs.getLoc(TAK_autoclosure),
diag::attr_not_on_variadic_parameters, "@autoclosure");
diagnoseInvalid(repr, attrs.getLoc(TAK_autoclosure),
diag::attr_not_on_variadic_parameters, "@autoclosure");
attrs.clearAttribute(TAK_autoclosure);
didDiagnose = true;
} else if (!options.is(TypeResolverContext::FunctionInput)) {
diagnose(attrs.getLoc(TAK_autoclosure), diag::attr_only_on_parameters,
"@autoclosure");
diagnoseInvalid(repr, attrs.getLoc(TAK_autoclosure),
diag::attr_only_on_parameters, "@autoclosure");
attrs.clearAttribute(TAK_autoclosure);
didDiagnose = true;
}
Expand Down Expand Up @@ -2318,12 +2331,13 @@ Type TypeResolver::resolveAttributedType(TypeAttributes &attrs,
auto loc = attrs.getLoc(TAK_escaping);
auto attrRange = getTypeAttrRangeWithAt(Context, loc);

diagnose(loc, diag::escaping_non_function_parameter)
.fixItRemove(attrRange);
diagnoseInvalid(repr, loc, diag::escaping_non_function_parameter)
.fixItRemove(attrRange);

// Try to find a helpful note based on how the type is being used
if (options.is(TypeResolverContext::ImmediateOptionalTypeArgument)) {
diagnose(repr->getLoc(), diag::escaping_optional_type_argument);
diagnoseInvalid(repr, repr->getLoc(),
diag::escaping_optional_type_argument);
}
}

Expand All @@ -2339,16 +2353,17 @@ Type TypeResolver::resolveAttributedType(TypeAttributes &attrs,
// @autoclosure is going to be diagnosed when type of
// the parameter is validated, because that attribute
// applies to the declaration now.
repr->setInvalid();
attrs.clearAttribute(TAK_autoclosure);
}

for (auto i : FunctionAttrs) {
if (!attrs.has(i))
continue;

auto diag = diagnose(attrs.getLoc(i),
diag::attribute_requires_function_type,
TypeAttributes::getAttrName(i));
auto diag = diagnoseInvalid(repr, attrs.getLoc(i),
diag::attribute_requires_function_type,
TypeAttributes::getAttrName(i));

// If we see @escaping among the attributes on this type, because it isn't
// a function type, we'll remove it.
Expand All @@ -2358,7 +2373,8 @@ Type TypeResolver::resolveAttributedType(TypeAttributes &attrs,
// Specialize the diagnostic for Optionals.
if (ty->getOptionalObjectType()) {
diag.flush();
diagnose(repr->getLoc(), diag::escaping_optional_type_argument);
diagnoseInvalid(repr, repr->getLoc(),
diag::escaping_optional_type_argument);
}
}
attrs.clearAttribute(i);
Expand All @@ -2373,7 +2389,8 @@ Type TypeResolver::resolveAttributedType(TypeAttributes &attrs,
// In SIL, handle @opened (n), which creates an existential archetype.
if (attrs.has(TAK_opened)) {
if (!ty->isExistentialType()) {
diagnose(attrs.getLoc(TAK_opened), diag::opened_non_protocol, ty);
diagnoseInvalid(repr, attrs.getLoc(TAK_opened), diag::opened_non_protocol,
ty);
} else {
ty = OpenedArchetypeType::get(ty, attrs.OpenedID);
}
Expand Down Expand Up @@ -2413,9 +2430,10 @@ Type TypeResolver::resolveAttributedType(TypeAttributes &attrs,
}

for (unsigned i = 0; i != TypeAttrKind::TAK_Count; ++i)
if (attrs.has((TypeAttrKind)i))
diagnose(attrs.getLoc((TypeAttrKind)i),
diag::attribute_does_not_apply_to_type);
if (attrs.has((TypeAttrKind)i)) {
diagnoseInvalid(repr, attrs.getLoc((TypeAttrKind)i),
diag::attribute_does_not_apply_to_type);
}

return ty;
}
Expand Down Expand Up @@ -3038,8 +3056,7 @@ Type TypeResolver::resolveSpecifierTypeRepr(SpecifierTypeRepr *repr,
default:
llvm_unreachable("unknown SpecifierTypeRepr kind");
}
diagnose(repr->getSpecifierLoc(), diagID, name);
repr->setInvalid();
diagnoseInvalid(repr, repr->getSpecifierLoc(), diagID, name);
return ErrorType::get(Context);
}

Expand Down
3 changes: 3 additions & 0 deletions test/attr/attr_autoclosure.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -292,3 +292,6 @@ func sr_11938_3(_ x: [@autoclosure String]) {} // expected-error {{'@autoclosure

protocol SR_11938_P {}
struct SR_11938_S : @autoclosure SR_11938_P {} // expected-error {{'@autoclosure' may only be used on parameters}}

// SR-9178
func bar<T>(_ x: @autoclosure T) {} // expected-error 1{{@autoclosure attribute only applies to function types}}
3 changes: 3 additions & 0 deletions test/attr/attr_escaping.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,3 +226,6 @@ extension SR_9760 {
func fiz<T>(_: T, _: @escaping F) {} // Ok
func baz<T>(_: @escaping G<T>) {} // Ok
}

// SR-9178
func foo<T>(_ x: @escaping T) {} // expected-error 1{{@escaping attribute only applies to function types}}
36 changes: 14 additions & 22 deletions test/attr/attr_noescape.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ func conflictingAttrs(_ fn: @noescape @escaping () -> Int) {} // expected-error

func doesEscape(_ fn : @escaping () -> Int) {}

func takesGenericClosure<T>(_ a : Int, _ fn : @noescape () -> T) {} // expected-error 2{{unknown attribute 'noescape'}}
func takesGenericClosure<T>(_ a : Int, _ fn : @noescape () -> T) {} // expected-error {{unknown attribute 'noescape'}}


var globalAny: Any = 0
Expand All @@ -23,8 +23,8 @@ func takesVariadic(_ fns: () -> Int...) {
doesEscape(fns[0]) // Okay - variadic-of-function parameters are escaping
}

func takesNoEscapeClosure(_ fn : () -> Int) { // expected-note 2 {{parameter 'fn' is implicitly non-escaping}} {{34-34=@escaping }}
// expected-note@-1 5{{parameter 'fn' is implicitly non-escaping}} {{34-34=@escaping }}
func takesNoEscapeClosure(_ fn : () -> Int) { // expected-note 1 {{parameter 'fn' is implicitly non-escaping}} {{34-34=@escaping }}
// expected-note@-1 6{{parameter 'fn' is implicitly non-escaping}} {{34-34=@escaping }}
takesNoEscapeClosure { 4 } // ok

_ = fn() // ok
Expand Down Expand Up @@ -194,10 +194,7 @@ func testAutoclosure(_ a : @autoclosure () -> Int) { // expected-note{{parameter


// <rdar://problem/19470858> QoI: @autoclosure implies @noescape, so you shouldn't be allowed to specify both
func redundant(_ fn : @noescape
@autoclosure () -> Int) {
// expected-error@-2{{unknown attribute 'noescape'}}
}
func redundant(_ fn : @noescape @autoclosure () -> Int) {} // expected-error {{unknown attribute 'noescape'}}


protocol P1 {
Expand All @@ -214,9 +211,7 @@ func overloadedEach<P: P2, T>(_ source: P, _ transform: @escaping (P.Element) ->
struct S : P2 {
typealias Element = Int
func each(_ transform: @noescape (Int) -> ()) { // expected-error{{unknown attribute 'noescape'}}
// expected-note@-1 {{parameter 'transform' is implicitly non-escaping}}
overloadedEach(self, transform, 1)
// expected-error@-1 {{passing non-escaping parameter 'transform' to function expecting an @escaping closure}}
}
}

Expand Down Expand Up @@ -260,22 +255,22 @@ public func XCTAssert(_ expression: @autoclosure () -> Bool, _ message: String =


/// SR-770 - Currying and `noescape`/`rethrows` don't work together anymore
func curriedFlatMap<A, B>(_ x: [A]) -> (@noescape (A) -> [B]) -> [B] { // expected-error 2{{unknown attribute 'noescape'}}
func curriedFlatMap<A, B>(_ x: [A]) -> (@noescape (A) -> [B]) -> [B] { // expected-error 1{{unknown attribute 'noescape'}}
return { f in
x.flatMap(f)
}
}

func curriedFlatMap2<A, B>(_ x: [A]) -> (@noescape (A) -> [B]) -> [B] { // expected-error 2{{unknown attribute 'noescape'}}
return { (f : @noescape (A) -> [B]) in // expected-error{{unknown attribute 'noescape'}}
func curriedFlatMap2<A, B>(_ x: [A]) -> (@noescape (A) -> [B]) -> [B] { // expected-error {{unknown attribute 'noescape'}}
return { (f : @noescape (A) -> [B]) in
x.flatMap(f)
}
}

func bad(_ a : @escaping (Int)-> Int) -> Int { return 42 }
func escapeNoEscapeResult(_ x: [Int]) -> (@noescape (Int) -> Int) -> Int { // expected-error{{unknown attribute 'noescape'}}
return { f in // expected-note {{parameter 'f' is implicitly non-escaping}}
bad(f) // expected-error {{passing non-escaping parameter 'f' to function expecting an @escaping closure}}
return { f in
bad(f)
}
}

Expand All @@ -295,24 +290,21 @@ typealias CompletionHandler = (_ success: Bool) -> ()
var escape : CompletionHandlerNE
var escapeOther : CompletionHandler
func doThing1(_ completion: (_ success: Bool) -> ()) {
// expected-note@-1 {{parameter 'completion' is implicitly non-escaping}}
escape = completion // expected-error {{assigning non-escaping parameter 'completion' to an @escaping closure}}
escape = completion
}
func doThing2(_ completion: CompletionHandlerNE) {
// expected-note@-1 {{parameter 'completion' is implicitly non-escaping}}
escape = completion // expected-error {{assigning non-escaping parameter 'completion' to an @escaping closure}}
escape = completion
}
func doThing3(_ completion: CompletionHandler) {
// expected-note@-1 {{parameter 'completion' is implicitly non-escaping}}
escape = completion // expected-error {{assigning non-escaping parameter 'completion' to an @escaping closure}}
escape = completion
}
func doThing4(_ completion: @escaping CompletionHandler) {
escapeOther = completion
}

// <rdar://problem/19997680> @noescape doesn't work on parameters of function type
func apply<T, U>(_ f: @noescape (T) -> U, g: @noescape (@noescape (T) -> U) -> U) -> U {
// expected-error@-1 6{{unknown attribute 'noescape'}}
// expected-error@-1 2{{unknown attribute 'noescape'}}
return g(f)
}

Expand All @@ -322,7 +314,7 @@ enum r19997577Type {
case Function(() -> r19997577Type, () -> r19997577Type)
case Sum(() -> r19997577Type, () -> r19997577Type)

func reduce<Result>(_ initial: Result, _ combine: @noescape (Result, r19997577Type) -> Result) -> Result { // expected-error 2{{unknown attribute 'noescape'}}
func reduce<Result>(_ initial: Result, _ combine: @noescape (Result, r19997577Type) -> Result) -> Result { // expected-error 1{{unknown attribute 'noescape'}}
let binary: @noescape (r19997577Type, r19997577Type) -> Result = { combine(combine(combine(initial, self), $0), $1) } // expected-error{{unknown attribute 'noescape'}}
switch self {
case .Unit:
Expand Down