Skip to content
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
6 changes: 5 additions & 1 deletion lib/Sema/CSFix.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -939,7 +939,11 @@ DefineMemberBasedOnUse::diagnoseForAmbiguity(CommonFixesArray commonFixes) const
const auto *solution = solutionAndFix.first;
const auto *fix = solutionAndFix.second->getAs<DefineMemberBasedOnUse>();

auto baseType = solution->simplifyType(fix->BaseType);
// Ignore differences in optionality since we can look through an optional
// to resolve an implicit member `.foo`.
auto baseType = solution->simplifyType(fix->BaseType)
->getMetatypeInstanceType()
->lookThroughAllOptionalTypes();
if (!concreteBaseType)
concreteBaseType = baseType;

Expand Down
11 changes: 7 additions & 4 deletions lib/Sema/CSGen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1308,14 +1308,17 @@ namespace {
CS.setType(expr, expansionType);
}

/// Records a fix for an invalid AST node, and returns a potential hole
/// type variable for it.
/// Records a fix for an invalid AST node, and returns a hole for it.
Type recordInvalidNode(ASTNode node) {
CS.recordFix(
IgnoreInvalidASTNode::create(CS, CS.getConstraintLocator(node)));

return CS.createTypeVariable(CS.getConstraintLocator(node),
TVO_CanBindToHole);
// Ideally we wouldn't need a type variable here, but we don't have a
// suitable placeholder originator for all the cases here.
auto ty = CS.createTypeVariable(CS.getConstraintLocator(node),
TVO_CanBindToHole);
CS.recordTypeVariablesAsHoles(ty);
return ty;
}

virtual Type visitErrorExpr(ErrorExpr *E) {
Expand Down
23 changes: 8 additions & 15 deletions lib/Sema/CSSimplify.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6694,6 +6694,10 @@ bool ConstraintSystem::repairFailures(
}

case ConstraintLocator::Condition: {
// If the condition is already a hole, we're done.
if (lhs->isPlaceholder())
return true;

if (repairViaOptionalUnwrap(*this, lhs, rhs, matchKind, conversionsOrFixes,
locator))
return true;
Expand Down Expand Up @@ -11604,22 +11608,11 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyMemberConstraint(
// `key path` constraint can't be retired until all components
// are simplified.
addTypeVariableConstraintsToWorkList(memberTypeVar);
} else if (isa<Expr *>(locator->getAnchor()) &&
!getSemanticsProvidingParentExpr(
getAsExpr(locator->getAnchor()))) {
// If there are no contextual expressions that could provide
// a type for the member type variable, let's default it to
// a placeholder eagerly so it could be propagated to the
// pattern if necessary.
recordTypeVariablesAsHoles(memberTypeVar);
} else if (locator->isLastElement<LocatorPathElt::PatternMatch>()) {
// Let's handle member patterns specifically because they use
// equality instead of argument application constraint, so allowing
// them to bind member could mean missing valid hole positions in
// the pattern.
recordTypeVariablesAsHoles(memberTypeVar);
} else {
recordPotentialHole(memberTypeVar);
// Eagerly turn the member type variable into a hole since we know
// this is where the issue is and we've recorded a fix for it. This
// avoids producing unnecessary holes for e.g generic parameters.
recordTypeVariablesAsHoles(memberTypeVar);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We didn't do this originally because sometimes it still helpful to wait and infer member type from the context to help avoid _ in diagnostics but that indeed leads to some unnecessary hole propagation. But we do want to be more aggressive with missing members so I think this is okay to try and see how it goes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I think we ought to be able to handle those cases in repairFailures instead

}
}

Expand Down
6 changes: 3 additions & 3 deletions lib/Sema/ConstraintSystem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3581,12 +3581,13 @@ void constraints::simplifyLocator(ASTNode &anchor,
}
case ConstraintLocator::AutoclosureResult:
case ConstraintLocator::LValueConversion:
case ConstraintLocator::OptionalInjection:
case ConstraintLocator::DynamicType:
case ConstraintLocator::UnresolvedMember:
case ConstraintLocator::ImplicitCallAsFunction:
// Arguments in autoclosure positions, lvalue and rvalue adjustments,
// unresolved members, and implicit callAsFunction references are
// implicit.
// optional injections, unresolved members, and implicit callAsFunction
// references are implicit.
path = path.slice(1);
continue;

Expand Down Expand Up @@ -3913,7 +3914,6 @@ void constraints::simplifyLocator(ASTNode &anchor,

case ConstraintLocator::Witness:
case ConstraintLocator::WrappedValue:
case ConstraintLocator::OptionalInjection:
case ConstraintLocator::ImplicitlyUnwrappedDisjunctionChoice:
case ConstraintLocator::FallbackType:
case ConstraintLocator::KeyPathSubscriptIndex:
Expand Down
2 changes: 1 addition & 1 deletion test/Constraints/closures.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1362,7 +1362,7 @@ do {
}
}

test { // expected-error {{invalid conversion from throwing function of type '(Int) throws -> Void' to non-throwing function type '(Int) -> Void'}}
test { // expected-error {{invalid conversion from throwing function of type '(Int) throws -> _' to non-throwing function type '(Int) -> Void'}}
try $0.missing // expected-error {{value of type 'Int' has no member 'missing'}}
}
}
Expand Down
6 changes: 4 additions & 2 deletions test/Constraints/diagnostics.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1126,10 +1126,12 @@ func rdar17170728() {
// expected-error@-1 4 {{optional type 'Int?' cannot be used as a boolean; test for '!= nil' instead}}
}

// FIXME: Bad diagnostic, `Bool.Stride` is bogus, we shouldn't be suggesting
// `reduce(into:)`, and the actual problem is that Int cannot be used as a boolean
// condition.
let _ = [i, j, k].reduce(0 as Int?) { // expected-error {{missing argument label 'into:' in call}}
// expected-error@-1 {{cannot convert value of type 'Int?' to expected argument type '(inout (Bool, Bool) -> Bool?, Int?) throws -> ()'}}
$0 && $1 ? $0 + $1 : ($0 ? $0 : ($1 ? $1 : nil))
// expected-error@-1 {{binary operator '+' cannot be applied to two 'Bool' operands}}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missed the fact that the diagnostic changed here, this is because the extra check for hole in repairFailures, previously we were recording a redundant fix and then declining to emit a diagnostic for it, so now we're picking a different overload of +. The original diagnostic wasn't very good anyway, so I'm going to leave this for a follow-up since it doesn't immediately look like a straightforward fix

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, this expression is very tricky to diagnose properly…

// expected-error@-1 {{binary operator '+' cannot be applied to operands of type 'Bool.Stride' and 'Bool'}}
}
}

Expand Down
4 changes: 4 additions & 0 deletions test/Constraints/ternary_expr.swift
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,10 @@ _ = true ? x : 1.2 // expected-error {{result values in '? :' expression have mi
_ = (x: true) ? true : false // expected-error {{cannot convert value of type '(x: Bool)' to expected condition type 'Bool'}}
_ = (x: 1) ? true : false // expected-error {{cannot convert value of type '(x: Int)' to expected condition type 'Bool'}}

_ = undefined ? 0 : 1 // expected-error {{cannot find 'undefined' in scope}}
_ = [undefined] ? 0 : 1 // expected-error {{cannot find 'undefined' in scope}}
// expected-error@-1 {{cannot convert value of type '[Element]' to expected condition type 'Bool'}}

func resultBool() -> Bool { true }
_ = resultBool ? true : false // expected-error {{function 'resultBool' was used as a property; add () to call it}} {{15-15=()}}

Expand Down
1 change: 0 additions & 1 deletion test/Parse/subscripting.swift
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,6 @@ struct A6 {
// expected-note@-2 {{did you mean}}
get {
return i + j // expected-error {{cannot find 'j' in scope}}
// expected-error@-1 {{cannot convert return expression of type 'Int' to return type '(Int) -> Int'}}
}
}
}
Expand Down
3 changes: 3 additions & 0 deletions test/stmt/statements.swift
Original file line number Diff line number Diff line change
Expand Up @@ -560,6 +560,9 @@ func bad_if() {
if (x: false) {} // expected-error {{cannot convert value of type '(x: Bool)' to expected condition type 'Bool'}}
if (x: 1) {} // expected-error {{cannot convert value of type '(x: Int)' to expected condition type 'Bool'}}
if nil {} // expected-error {{'nil' is not compatible with expected condition type 'Bool'}}
if undefined {} // expected-error {{cannot find 'undefined' in scope}}
if [undefined] {} // expected-error {{cannot find 'undefined' in scope}}
// expected-error@-1 {{cannot convert value of type '[Element]' to expected condition type 'Bool'}}
}

// Typo correction for loop labels
Expand Down
16 changes: 6 additions & 10 deletions test/type/protocol_composition.swift
Original file line number Diff line number Diff line change
Expand Up @@ -191,16 +191,12 @@ takesP1AndP2([AnyObject & P1 & P2]())
takesP1AndP2([Swift.AnyObject & P1 & P2]())
takesP1AndP2([AnyObject & protocol_composition.P1 & P2]())
takesP1AndP2([AnyObject & P1 & protocol_composition.P2]())
takesP1AndP2([DoesNotExist & P1 & P2]()) // expected-error {{cannot find 'DoesNotExist' in scope}}
// expected-error@-1 {{binary operator '&' cannot be applied to operands of type 'UInt8' and '(any P2).Type'}}
// expected-error@-2 {{binary operator '&' cannot be applied to operands of type 'UInt8' and '(any P1).Type'}}
// expected-note@-3 2 {{overloads for '&' exist with these partially matching parameter lists}}
// expected-error@-4 {{cannot call value of non-function type '[UInt8]'}}
takesP1AndP2([Swift.DoesNotExist & P1 & P2]()) // expected-error {{module 'Swift' has no member named 'DoesNotExist'}}
// expected-error@-1 {{binary operator '&' cannot be applied to operands of type 'UInt8' and '(any P2).Type'}}
// expected-error@-2 {{binary operator '&' cannot be applied to operands of type 'UInt8' and '(any P1).Type'}}
// expected-note@-3 2 {{overloads for '&' exist with these partially matching parameter lists}}
// expected-error@-4 {{cannot call value of non-function type '[UInt8]'}}
takesP1AndP2([DoesNotExist & P1 & P2]())
// expected-error@-1 {{cannot find 'DoesNotExist' in scope}}
// expected-error@-2 {{cannot call value of non-function type '[Element]'}}
takesP1AndP2([Swift.DoesNotExist & P1 & P2]())
// expected-error@-1 {{module 'Swift' has no member named 'DoesNotExist'}}
// expected-error@-2 {{cannot call value of non-function type '[Element]'}}

typealias T08 = P1 & inout P2 // expected-error {{'inout' may only be used on parameters}}
typealias T09 = P1 & __shared P2 // expected-error {{'__shared' may only be used on parameters}}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// {"kind":"typecheck","signature":"(anonymous namespace)::ConnectedComponents::unionSets(swift::TypeVariableType*, swift::TypeVariableType*)","signatureAssert":"Assertion failed: (validComponentCount > 0), function unionSets"}
// RUN: not --crash %target-swift-frontend -typecheck %s
// RUN: not %target-swift-frontend -typecheck %s
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The underlying crashes here aren't really fixed, I've added some similar variants as part of #85407, and the fuzzer should hopefully pick up more cases for the rest

let b = ( c , d a {
e b
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// {"kind":"typecheck","signature":"swift::constraints::ConstraintGraph::computeConnectedComponents(llvm::ArrayRef<swift::TypeVariableType*>)","signatureAssert":"Assertion failed: (component != components.end()), function operator()"}
// RUN: not --crash %target-swift-frontend -typecheck %s
// RUN: not %target-swift-frontend -typecheck %s
let i = Array(... a { " ? \(i Array* )" }
, b 1
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// {"kind":"typecheck","original":"1562769e","signature":"(anonymous namespace)::ConstraintWalker::walkToExprPost(swift::Expr*)"}
// RUN: not --crash %target-swift-frontend -typecheck %s
// RUN: not %target-swift-frontend -typecheck %s
enum c
func d() e {
if let
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// {"kind":"typecheck","signature":"(anonymous namespace)::ExprRewriter::visitDeclRefExpr(swift::DeclRefExpr*)"}
// RUN: not --crash %target-swift-frontend -typecheck %s
// RUN: not %target-swift-frontend -typecheck %s
{
for b 0 ..< 10 {
let a = Array(0 ..< b)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
// {"kind":"typecheck","signature":"swift::PackType::getExpandedGenericArgs(llvm::ArrayRef<swift::GenericTypeParamType*>, llvm::ArrayRef<swift::Type>)","signatureAssert":"Assertion failed: (isa<To>(Val) && \"cast<Ty>() argument of incompatible type!\"), function cast"}
// RUN: not --crash %target-swift-frontend -typecheck %s
// RUN: not %target-swift-frontend -typecheck %s
struct a < each b typealias c<each d> = a<> c < e