Skip to content

[Typechecker] Fix fix-it location for missing try when called on optional protocol value #26606

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 6 commits into from
Aug 16, 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
11 changes: 6 additions & 5 deletions include/swift/AST/Expr.h
Original file line number Diff line number Diff line change
Expand Up @@ -3843,11 +3843,12 @@ class DynamicTypeExpr : public Expr {
/// node (say, an \c OpenExistentialExpr) and can only be used within the
/// subexpressions of that AST node.
class OpaqueValueExpr : public Expr {
SourceLoc Loc;
SourceRange Range;

public:
explicit OpaqueValueExpr(SourceLoc Loc, Type Ty, bool isPlaceholder = false)
: Expr(ExprKind::OpaqueValue, /*Implicit=*/true, Ty), Loc(Loc) {
explicit OpaqueValueExpr(SourceRange Range, Type Ty,
bool isPlaceholder = false)
: Expr(ExprKind::OpaqueValue, /*Implicit=*/true, Ty), Range(Range) {
Bits.OpaqueValueExpr.IsPlaceholder = isPlaceholder;
}

Expand All @@ -3856,8 +3857,8 @@ class OpaqueValueExpr : public Expr {
/// value to be specified later.
bool isPlaceholder() const { return Bits.OpaqueValueExpr.IsPlaceholder; }

SourceRange getSourceRange() const { return Loc; }
SourceRange getSourceRange() const { return Range; }

static bool classof(const Expr *E) {
return E->getKind() == ExprKind::OpaqueValue;
}
Expand Down
63 changes: 27 additions & 36 deletions lib/Sema/CSApply.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -668,7 +668,8 @@ namespace {
opaqueType = LValueType::get(opaqueType);

ASTContext &ctx = tc.Context;
auto archetypeVal = new (ctx) OpaqueValueExpr(base->getLoc(), opaqueType);
auto archetypeVal =
new (ctx) OpaqueValueExpr(base->getSourceRange(), opaqueType);
cs.cacheType(archetypeVal);

// Record the opened existential.
Expand Down Expand Up @@ -2169,8 +2170,8 @@ namespace {

// This OpaqueValueExpr represents the result of builderInit above in
// silgen.
OpaqueValueExpr *interpolationExpr =
new (tc.Context) OpaqueValueExpr(expr->getLoc(), interpolationType);
OpaqueValueExpr *interpolationExpr = new (tc.Context)
OpaqueValueExpr(expr->getSourceRange(), interpolationType);
cs.setType(interpolationExpr, interpolationType);
expr->setInterpolationExpr(interpolationExpr);

Expand Down Expand Up @@ -5043,8 +5044,8 @@ Expr *ExprRewriter::coerceTupleToTuple(Expr *expr,
SmallVector<OpaqueValueExpr *, 4> destructured;
for (unsigned i = 0, e = sources.size(); i != e; ++i) {
auto fromEltType = fromTuple->getElementType(i);
auto *opaqueElt = new (tc.Context) OpaqueValueExpr(expr->getLoc(),
fromEltType);
auto *opaqueElt =
new (tc.Context) OpaqueValueExpr(expr->getSourceRange(), fromEltType);
cs.cacheType(opaqueElt);
destructured.push_back(opaqueElt);
}
Expand Down Expand Up @@ -5138,10 +5139,8 @@ Expr *ExprRewriter::coerceSuperclass(Expr *expr, Type toType,
// concrete superclass.
auto fromArchetype = OpenedArchetypeType::getAny(fromType);

auto *archetypeVal =
cs.cacheType(
new (tc.Context) OpaqueValueExpr(expr->getLoc(),
fromArchetype));
auto *archetypeVal = cs.cacheType(new (tc.Context) OpaqueValueExpr(
expr->getSourceRange(), fromArchetype));

auto *result = coerceSuperclass(archetypeVal, toType, locator);

Expand Down Expand Up @@ -5204,12 +5203,10 @@ Expr *ExprRewriter::coerceExistential(Expr *expr, Type toType,
// For existential-to-existential coercions, open the source existential.
if (fromType->isAnyExistentialType()) {
fromType = OpenedArchetypeType::getAny(fromType);

auto *archetypeVal =
cs.cacheType(
new (ctx) OpaqueValueExpr(expr->getLoc(),
fromType));


auto *archetypeVal = cs.cacheType(
new (ctx) OpaqueValueExpr(expr->getSourceRange(), fromType));

auto *result = cs.cacheType(ErasureExpr::create(ctx, archetypeVal, toType,
conformances));
return cs.cacheType(
Expand Down Expand Up @@ -5894,10 +5891,8 @@ maybeDiagnoseUnsupportedFunctionConversion(ConstraintSystem &cs, Expr *expr,

/// Build the conversion of an element in a collection upcast.
static Expr *buildElementConversion(ExprRewriter &rewriter,
SourceLoc srcLoc,
Type srcType,
Type destType,
bool bridged,
SourceRange srcRange, Type srcType,
Type destType, bool bridged,
ConstraintLocatorBuilder locator,
Expr *element) {
auto &cs = rewriter.getConstraintSystem();
Expand All @@ -5917,12 +5912,9 @@ static Expr *buildElementConversion(ExprRewriter &rewriter,
}

static CollectionUpcastConversionExpr::ConversionPair
buildOpaqueElementConversion(ExprRewriter &rewriter,
SourceLoc srcLoc,
Type srcCollectionType,
Type destCollectionType,
bool bridged,
ConstraintLocatorBuilder locator,
buildOpaqueElementConversion(ExprRewriter &rewriter, SourceRange srcRange,
Type srcCollectionType, Type destCollectionType,
bool bridged, ConstraintLocatorBuilder locator,
unsigned typeArgIndex) {
// We don't need this stuff unless we've got generalized casts.
Type srcType = srcCollectionType->castTo<BoundGenericType>()
Expand All @@ -5934,14 +5926,13 @@ buildOpaqueElementConversion(ExprRewriter &rewriter,
auto &cs = rewriter.getConstraintSystem();
ASTContext &ctx = cs.getASTContext();
auto opaque =
rewriter.cs.cacheType(new (ctx) OpaqueValueExpr(srcLoc, srcType));
rewriter.cs.cacheType(new (ctx) OpaqueValueExpr(srcRange, srcType));

Expr *conversion =
buildElementConversion(rewriter, srcLoc, srcType, destType, bridged,
locator.withPathElement(
ConstraintLocator::PathElement::getGenericArgument(
typeArgIndex)),
opaque);
Expr *conversion = buildElementConversion(
rewriter, srcRange, srcType, destType, bridged,
locator.withPathElement(
ConstraintLocator::PathElement::getGenericArgument(typeArgIndex)),
opaque);

return { opaque, conversion };
}
Expand Down Expand Up @@ -6996,9 +6987,9 @@ Expr *ExprRewriter::finishApply(ApplyExpr *apply, Type openedType,
bodyFnTy->withExtInfo(bodyFnTy->getExtInfo().withNoEscape()));
body = coerceToType(body, bodyFnTy, locator);
assert(body && "can't make nonescaping?!");

auto escapable = new (tc.Context)
OpaqueValueExpr(apply->getFn()->getLoc(), Type());
OpaqueValueExpr(apply->getFn()->getSourceRange(), Type());
cs.setType(escapable, escapableParams[0].getOldType());

auto getType = [&](const Expr *E) -> Type {
Expand Down Expand Up @@ -7055,8 +7046,8 @@ Expr *ExprRewriter::finishApply(ApplyExpr *apply, Type openedType,
->getOpenedExistentialType()
->isEqual(existentialInstanceTy));

auto opaqueValue = new (tc.Context)
OpaqueValueExpr(apply->getLoc(), openedTy);
auto opaqueValue =
new (tc.Context) OpaqueValueExpr(apply->getSourceRange(), openedTy);
cs.setType(opaqueValue, openedTy);

auto getType = [&](const Expr *E) -> Type {
Expand Down
2 changes: 1 addition & 1 deletion lib/Sema/TypeCheckStorage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2446,7 +2446,7 @@ PropertyWrapperBackingPropertyInfoRequest::evaluate(Evaluator &evaluator,
// Form the initialization of the backing property from a value of the
// original property's type.
OpaqueValueExpr *origValue =
new (ctx) OpaqueValueExpr(var->getLoc(), var->getType(),
new (ctx) OpaqueValueExpr(var->getSourceRange(), var->getType(),
/*isPlaceholder=*/true);
Expr *initializer = buildPropertyWrapperInitialValueCall(
var, storageType, origValue,
Expand Down
17 changes: 17 additions & 0 deletions test/decl/func/throwing_functions_without_try.swift
Original file line number Diff line number Diff line change
Expand Up @@ -51,3 +51,20 @@ func baz2() -> Int {
x = try foo() // expected-error{{errors thrown from here are not handled}}
return x
}

// SR-11016

protocol SR_11016_P {
func bar() throws
}

class SR_11016_C {
var foo: SR_11016_P?

func someMethod() throws {
foo?.bar() // expected-error {{call can throw but is not marked with 'try'}}
// expected-note @-1 {{did you mean to use 'try'?}}{{5-5=try }}
// expected-note @-2 {{did you mean to handle error as optional value?}}{{5-5=try? }}
// expected-note @-3 {{did you mean to disable error propagation?}}{{5-5=try! }}
}
}