Skip to content

[CSSimplify] Detect and diagnose conformance failures related to AnyHashable conversion #65806

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.

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 5 commits into from
May 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
49 changes: 49 additions & 0 deletions lib/Sema/CSDiagnostics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include "swift/AST/DiagnosticsClangImporter.h"
#include "swift/AST/ExistentialLayout.h"
#include "swift/AST/Expr.h"
#include "swift/AST/GenericEnvironment.h"
#include "swift/AST/GenericSignature.h"
#include "swift/AST/ImportCache.h"
#include "swift/AST/Initializer.h"
Expand Down Expand Up @@ -88,6 +89,54 @@ Type FailureDiagnostic::getRawType(ASTNode node) const {
return S.getType(node);
}

Type FailureDiagnostic::resolveType(Type rawType, bool reconstituteSugar,
bool wantRValue) const {
rawType = rawType.transform([&](Type type) -> Type {
if (auto *typeVar = type->getAs<TypeVariableType>()) {
auto resolvedType = S.simplifyType(typeVar);

if (!resolvedType->hasUnresolvedType())
return resolvedType;

// If type variable was simplified to an unresolved pack expansion
// type, let's examine its original pattern type because it could
// contain type variables replaceable with their generic parameter
// types.
if (auto *expansion = resolvedType->getAs<PackExpansionType>()) {
auto *locator = typeVar->getImpl().getLocator();
auto *openedExpansionTy =
locator->castLastElementTo<LocatorPathElt::PackExpansionType>()
.getOpenedType();
auto patternType = resolveType(openedExpansionTy->getPatternType());
return PackExpansionType::get(patternType, expansion->getCountType());
}

Type GP = typeVar->getImpl().getGenericParameter();
return resolvedType->is<UnresolvedType>() && GP ? GP : resolvedType;
}

if (type->hasElementArchetype()) {
auto *env = getDC()->getGenericEnvironmentOfContext();
return env->mapElementTypeIntoPackContext(type);
}

if (auto *packType = type->getAs<PackType>()) {
if (packType->getNumElements() == 1) {
auto eltType = resolveType(packType->getElementType(0));
if (auto expansion = eltType->getAs<PackExpansionType>())
return expansion->getPatternType();
}
}

return type->isPlaceholder() ? Type(type->getASTContext().TheUnresolvedType)
: type;
});

if (reconstituteSugar)
rawType = rawType->reconstituteSugar(/*recursive*/ true);
return wantRValue ? rawType->getRValueType() : rawType;
}

template <typename... ArgTypes>
InFlightDiagnostic
FailureDiagnostic::emitDiagnostic(ArgTypes &&... Args) const {
Expand Down
43 changes: 1 addition & 42 deletions lib/Sema/CSDiagnostics.h
Original file line number Diff line number Diff line change
Expand Up @@ -98,48 +98,7 @@ class FailureDiagnostic {

/// Resolve type variables present in the raw type, if any.
Type resolveType(Type rawType, bool reconstituteSugar = false,
bool wantRValue = true) const {
rawType = rawType.transform([&](Type type) -> Type {
if (auto *typeVar = type->getAs<TypeVariableType>()) {
auto resolvedType = S.simplifyType(typeVar);

if (!resolvedType->hasUnresolvedType())
return resolvedType;

// If type variable was simplified to an unresolved pack expansion
// type, let's examine its original pattern type because it could
// contain type variables replaceable with their generic parameter
// types.
if (auto *expansion = resolvedType->getAs<PackExpansionType>()) {
auto *locator = typeVar->getImpl().getLocator();
auto *openedExpansionTy =
locator->castLastElementTo<LocatorPathElt::PackExpansionType>()
.getOpenedType();
auto patternType = resolveType(openedExpansionTy->getPatternType());
return PackExpansionType::get(patternType, expansion->getCountType());
}

Type GP = typeVar->getImpl().getGenericParameter();
return resolvedType->is<UnresolvedType>() && GP ? GP : resolvedType;
}

if (auto *packType = type->getAs<PackType>()) {
if (packType->getNumElements() == 1) {
auto eltType = resolveType(packType->getElementType(0));
if (auto expansion = eltType->getAs<PackExpansionType>())
return expansion->getPatternType();
}
}

return type->isPlaceholder()
? Type(type->getASTContext().TheUnresolvedType)
: type;
});

if (reconstituteSugar)
rawType = rawType->reconstituteSugar(/*recursive*/ true);
return wantRValue ? rawType->getRValueType() : rawType;
}
bool wantRValue = true) const;

template <typename... ArgTypes>
InFlightDiagnostic emitDiagnostic(ArgTypes &&... Args) const;
Expand Down
26 changes: 26 additions & 0 deletions lib/Sema/CSSimplify.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2785,6 +2785,23 @@ assessRequirementFailureImpact(ConstraintSystem &cs, Type requirementType,
impact += choiceImpact - 1;
}
}

// If this requirement is associated with a call that is itself
// incorrect, let's increase impact to indicate that this failure
// has a compounding effect on viability of the overload choice it
// comes from.
if (locator.endsWith<LocatorPathElt::AnyRequirement>()) {
if (auto *expr = getAsExpr(anchor)) {
if (auto *call = getAsExpr<ApplyExpr>(cs.getParentExpr(expr))) {
if (call->getFn() == expr &&
llvm::any_of(cs.getFixes(), [&](const auto &fix) {
return getAsExpr(fix->getAnchor()) == call;
}))
impact += 2;
}
}
}

return impact;
}

Expand Down Expand Up @@ -8409,6 +8426,15 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyConformsToConstraint(
return SolutionKind::Solved;
}

// Conformance constraint that is introduced by an implicit conversion
// for example to `AnyHashable`.
if (kind == ConstraintKind::ConformsTo &&
loc->isLastElement<LocatorPathElt::ApplyArgToParam>()) {
auto *fix = AllowArgumentMismatch::create(*this, type, protocolTy, loc);
return recordFix(fix, /*impact=*/2) ? SolutionKind::Error
: SolutionKind::Solved;
}

// If this is an implicit Hashable conformance check generated for each
// index argument of the keypath subscript component, we could just treat
// it as though it conforms.
Expand Down
25 changes: 22 additions & 3 deletions lib/Sema/ConstraintSystem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4338,9 +4338,28 @@ static void diagnoseOperatorAmbiguity(ConstraintSystem &cs,
if (overloadType->hasTypeVariable())
continue;

if (auto *fnType = overloadType->getAs<FunctionType>())
parameters.insert(
FunctionType::getParamListAsString(fnType->getParams()));
auto overloadFnTy = overloadType->getAs<FunctionType>();
if (!overloadFnTy)
continue;

// If arguments to all parameters have been fixed then there is nothing
// to note about in this overload.
std::set<unsigned> fixedParams;
llvm::for_each(solution.Fixes, [&](const ConstraintFix *fix) {
auto *locator = fix->getLocator();
if (getAsExpr(locator->getAnchor()) != applyExpr)
return;

if (auto argLoc = locator->findLast<LocatorPathElt::ApplyArgToParam>()) {
fixedParams.insert(argLoc->getParamIdx());
}
});

if (fixedParams.size() == overloadFnTy->getNumParams())
continue;

parameters.insert(
FunctionType::getParamListAsString(overloadFnTy->getParams()));
}

// All of the overload choices had generic parameters like `Self`.
Expand Down
1 change: 0 additions & 1 deletion test/Constraints/bridging.swift
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,6 @@ func rdar19831698() {
var v71 = true + 1.0 // expected-error{{binary operator '+' cannot be applied to operands of type 'Bool' and 'Double'}}
// expected-note@-1{{overloads for '+'}}
var v72 = true + true // expected-error{{binary operator '+' cannot be applied to two 'Bool' operands}}
// expected-note@-1{{overloads for '+'}}
var v73 = true + [] // expected-error@:13 {{cannot convert value of type 'Bool' to expected argument type 'Array<Bool>'}}
var v75 = true + "str" // expected-error@:13 {{cannot convert value of type 'Bool' to expected argument type 'String'}}
}
Expand Down
12 changes: 9 additions & 3 deletions test/Constraints/diagnostics.swift
Original file line number Diff line number Diff line change
Expand Up @@ -661,10 +661,7 @@ example21890157.property = "confusing" // expected-error {{value of optional ty


struct UnaryOp {}

_ = -UnaryOp() // expected-error {{unary operator '-' cannot be applied to an operand of type 'UnaryOp'}}
// expected-note@-1 {{overloads for '-' exist with these partially matching parameter lists: (Double), (Float)}}


// <rdar://problem/23433271> Swift compiler segfault in failure diagnosis
func f23433271(_ x : UnsafePointer<Int>) {}
Expand Down Expand Up @@ -1551,3 +1548,12 @@ func rdar86611718(list: [Int]) {
String(list.count())
// expected-error@-1 {{cannot call value of non-function type 'Int'}}
}

// rdar://108977234 - failed to produce diagnostic when argument to AnyHashable parameter doesn't conform to Hashable protocol
do {
struct NonHashable {}

func test(result: inout [AnyHashable], value: NonHashable) {
result.append(value) // expected-error {{argument type 'NonHashable' does not conform to expected type 'Hashable'}}
}
}
1 change: 0 additions & 1 deletion test/Constraints/fixes.swift
Original file line number Diff line number Diff line change
Expand Up @@ -363,5 +363,4 @@ func testKeyPathSubscriptArgFixes(_ fn: @escaping () -> Int) {
// https://github.com/apple/swift/issues/54865
func f_54865(a: Any, _ str: String?) {
a == str // expected-error {{binary operator '==' cannot be applied to operands of type 'Any' and 'String?'}}
// expected-note@-1 {{overloads for '==' exist with these partially matching parameter lists: (String, String)}}
}
1 change: 1 addition & 0 deletions test/Constraints/generics.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1017,6 +1017,7 @@ func test_requirement_failures_in_ambiguous_context() {
func f1<T: Equatable>(_: T, _: T) {} // expected-note {{where 'T' = 'A'}}

f1(A(), B()) // expected-error {{local function 'f1' requires that 'A' conform to 'Equatable'}}
// expected-error@-1 {{cannot convert value of type 'B' to expected argument type 'A'}}

func f2<T: P_56173, U: P_56173>(_: T, _: U) {}
// expected-note@-1 {{candidate requires that 'B' conform to 'P_56173' (requirement specified as 'U' : 'P_56173')}}
Expand Down
4 changes: 3 additions & 1 deletion test/Constraints/operator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,9 @@ func rdar46459603() {
var arr = ["key": e]

_ = arr.values == [e]
// expected-error@-1 {{binary operator '==' cannot be applied to operands of type 'Dictionary<String, E>.Values' and '[E]'}}
// expected-error@-1 {{referencing operator function '==' on 'Equatable' requires that 'Dictionary<String, E>.Values' conform to 'Equatable'}}
// expected-error@-2 {{cannot convert value of type '[E]' to expected argument type 'Dictionary<String, E>.Values'}}

_ = [arr.values] == [[e]]
// expected-error@-1 {{referencing operator function '==' on 'Array' requires that 'E' conform to 'Equatable'}} expected-note@-1 {{binary operator '==' cannot be synthesized for enums with associated values}}
// expected-error@-2 {{cannot convert value of type 'Dictionary<String, E>.Values' to expected element type '[E]'}}
Expand Down
1 change: 0 additions & 1 deletion test/Constraints/optional.swift
Original file line number Diff line number Diff line change
Expand Up @@ -436,7 +436,6 @@ func test_force_unwrap_not_being_too_eager() {
func invalidOptionalChaining(a: Any) {
a == "="? // expected-error {{cannot use optional chaining on non-optional value of type 'String'}}
// expected-error@-1 {{binary operator '==' cannot be applied to operands of type 'Any' and 'String?'}}
// expected-note@-2 {{overloads for '==' exist}}
}

/// https://github.com/apple/swift/issues/54739
Expand Down
15 changes: 11 additions & 4 deletions test/Constraints/pack-expansion-expressions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -66,18 +66,16 @@ func outerArchetype<each T, U>(t: repeat each T, u: U) where repeat each T: P {
func sameElement<each T, U>(t: repeat each T, u: U) where repeat each T: P, repeat each T == U {
// expected-error@-1{{same-element requirements are not yet supported}}

// FIXME: Opened element archetypes in diagnostics
let _: (repeat each T) = (repeat (each t).f(u))
// expected-error@-1 {{cannot convert value of type 'U' to expected argument type 'τ_1_0'}}
// expected-error@-1 {{cannot convert value of type 'U' to expected argument type 'each T'}}
}

func forEachEach<each C, U>(c: repeat each C, function: (U) -> Void)
where repeat each C: Collection, repeat (each C).Element == U {
// expected-error@-1{{same-element requirements are not yet supported}}

// FIXME: Opened element archetypes in diagnostics
_ = (repeat (each c).forEach(function))
// expected-error@-1 {{cannot convert value of type '(U) -> Void' to expected argument type '(τ_1_0.Element) throws -> Void'}}
// expected-error@-1 {{cannot convert value of type '(U) -> Void' to expected argument type '(each C.Element) throws -> Void'}}
}

func typeReprPacks<each T: ExpressibleByIntegerLiteral>(_ t: repeat each T) {
Expand Down Expand Up @@ -334,6 +332,15 @@ func test_pack_expansions_with_closures() {
takesVariadicFunction { y in fn(x, y) } // Ok
takesVariadicFunction { y, z in fn(y, z) } // Ok
}

// rdar://108977234 - invalid error non-pack type instead of missing `Hashable` conformance
func testEscapingCapture<each T>(_ t: repeat each T) -> () -> [AnyHashable] {
return {
var result = [AnyHashable]()
repeat result.append(each t) // expected-error {{argument type 'each T' does not conform to expected type 'Hashable'}}
return result
}
}
}

// rdar://107151854 - crash on invalid due to specialized pack expansion
Expand Down
1 change: 0 additions & 1 deletion test/StringProcessing/Parse/forward-slash-regex.swift
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,6 @@ _ = /x/.../y/

_ = /x/...
// expected-error@-1 {{unary operator '...' cannot be applied to an operand of type 'Regex<Substring>'}}
// expected-note@-2 {{overloads for '...' exist with these partially matching parameter lists}}

do {
_ = /x /...
Expand Down
3 changes: 0 additions & 3 deletions test/stdlib/UnicodeScalarDiagnostics.swift
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,8 @@ func test_UnicodeScalarDoesNotImplementArithmetic(_ us: UnicodeScalar, i: Int) {
isString(&a1)
// We don't check for the overload choices list on the overload note match because they may change on different platforms.
let a2 = "a" - "b" // expected-error {{binary operator '-' cannot be applied to two 'String' operands}}
// expected-note@-1 {{overloads for '-' exist with these partially matching parameter lists:}}
let a3 = "a" * "b" // expected-error {{binary operator '*' cannot be applied to two 'String' operands}}
// expected-note@-1 {{overloads for '*' exist with these partially matching parameter lists:}}
let a4 = "a" / "b" // expected-error {{binary operator '/' cannot be applied to two 'String' operands}}
// expected-note@-1 {{overloads for '/' exist with these partially matching parameter lists:}}

let b1 = us + us // expected-error {{binary operator '+' cannot be applied to two 'UnicodeScalar' (aka 'Unicode.Scalar') operands}}
let b2 = us - us // expected-error {{binary operator '-' cannot be applied to two 'UnicodeScalar' (aka 'Unicode.Scalar') operands}}
Expand Down