Skip to content

[CSFix] Delay missing unwrap locator simplification until diagnostic #26376

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 1 commit into from
Jul 29, 2019
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
8 changes: 8 additions & 0 deletions lib/Sema/CSDiagnostics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1085,6 +1085,14 @@ bool MissingOptionalUnwrapFailure::diagnoseAsError() {

auto *anchor = getAnchor();

// If this is an unresolved member expr e.g. `.foo` its
// base type is going to be the same as result type minus
// r-value adjustment because base could be an l-value type.
// We want to fix both cases by only diagnose one of them,
// otherwise this is just going to result in a duplcate diagnostic.
if (getLocator()->isLastElement(ConstraintLocator::UnresolvedMember))
return false;

if (auto assignExpr = dyn_cast<AssignExpr>(anchor))
anchor = assignExpr->getSrc();

Expand Down
6 changes: 2 additions & 4 deletions lib/Sema/CSFix.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -67,17 +67,15 @@ ForceDowncast *ForceDowncast::create(ConstraintSystem &cs, Type toType,

bool ForceOptional::diagnose(Expr *root, bool asNote) const {
MissingOptionalUnwrapFailure failure(root, getConstraintSystem(), BaseType,
UnwrappedType, FullLocator);
UnwrappedType, getLocator());
return failure.diagnose(asNote);
}

ForceOptional *ForceOptional::create(ConstraintSystem &cs, Type baseType,
Type unwrappedType,
ConstraintLocator *locator) {
auto *simplifiedLocator =
cs.getConstraintLocator(simplifyLocatorToAnchor(cs, locator));
return new (cs.getAllocator())
ForceOptional(cs, baseType, unwrappedType, simplifiedLocator, locator);
ForceOptional(cs, baseType, unwrappedType, locator);
}

bool UnwrapOptionalBase::diagnose(Expr *root, bool asNote) const {
Expand Down
9 changes: 3 additions & 6 deletions lib/Sema/CSFix.h
Original file line number Diff line number Diff line change
Expand Up @@ -255,14 +255,11 @@ class ForceDowncast final : public ConstraintFix {
class ForceOptional final : public ConstraintFix {
Type BaseType;
Type UnwrappedType;
ConstraintLocator *FullLocator;

ForceOptional(ConstraintSystem &cs, Type baseType, Type unwrappedType,
ConstraintLocator *simplifiedLocator,
ConstraintLocator *fullLocator)
: ConstraintFix(cs, FixKind::ForceOptional, simplifiedLocator),
BaseType(baseType), UnwrappedType(unwrappedType),
FullLocator(fullLocator) {
ConstraintLocator *locator)
: ConstraintFix(cs, FixKind::ForceOptional, locator), BaseType(baseType),
UnwrappedType(unwrappedType) {
assert(baseType && "Base type must not be null");
assert(unwrappedType && "Unwrapped type must not be null");
}
Expand Down
12 changes: 6 additions & 6 deletions lib/Sema/ConstraintLocator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -170,22 +170,22 @@ bool ConstraintLocator::isForKeyPathComponent() const {
});
}

static bool isLastElement(const ConstraintLocator *locator,
ConstraintLocator::PathElementKind expectedKind) {
auto path = locator->getPath();
bool ConstraintLocator::isLastElement(
ConstraintLocator::PathElementKind expectedKind) const {
auto path = getPath();
return !path.empty() && path.back().getKind() == expectedKind;
}

bool ConstraintLocator::isForGenericParameter() const {
return isLastElement(this, ConstraintLocator::GenericParameter);
return isLastElement(ConstraintLocator::GenericParameter);
}

bool ConstraintLocator::isForSequenceElementType() const {
return isLastElement(this, ConstraintLocator::SequenceElementType);
return isLastElement(ConstraintLocator::SequenceElementType);
}

bool ConstraintLocator::isForContextualType() const {
return isLastElement(this, ConstraintLocator::ContextualType);
return isLastElement(ConstraintLocator::ContextualType);
}

GenericTypeParamType *ConstraintLocator::getGenericParameter() const {
Expand Down
4 changes: 4 additions & 0 deletions lib/Sema/ConstraintLocator.h
Original file line number Diff line number Diff line change
Expand Up @@ -593,6 +593,10 @@ class ConstraintLocator : public llvm::FoldingSetNode {
/// Determine whether this locator points to the contextual type.
bool isForContextualType() const;

/// Check whether the last element in the path of this locator
/// is of a given kind.
bool isLastElement(ConstraintLocator::PathElementKind kind) const;

/// If this locator points to generic parameter return its type.
GenericTypeParamType *getGenericParameter() const;

Expand Down
16 changes: 16 additions & 0 deletions test/Constraints/optional.swift
Original file line number Diff line number Diff line change
Expand Up @@ -357,3 +357,19 @@ struct S {
}
}
}

// rdar://problem/53238058 - Crash in getCalleeLocator while trying to produce a diagnostic about missing optional unwrap
// associated with an argument to a call

func rdar_53238058() {
struct S {
init(_: Double) {}
init<T>(_ value: T) where T : BinaryFloatingPoint {}
}

func test(_ str: String) {
_ = S(Double(str)) // expected-error {{value of optional type 'Double?' must be unwrapped to a value of type 'Double'}}
// expected-note@-1 {{coalesce using '??' to provide a default when the optional value contains 'nil'}}
// expected-note@-2 {{force-unwrap using '!' to abort execution if the optional value contains 'nil'}}
}
}