Skip to content

[Sema][CSApply] Be more conservative while emitting always true conversion diagnostic #34980

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 12 commits into from
Feb 10, 2021
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
7 changes: 7 additions & 0 deletions include/swift/AST/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -1019,6 +1019,13 @@ WARNING(isa_is_always_true,none, "'%0' test is always true",
WARNING(isa_is_foreign_check,none,
"'is' test is always true because %0 is a Core Foundation type",
(Type))
WARNING(checked_cast_not_supported,none,
"runtime conversion from %0 to %1 is not supported; "
"%select{'is' test|cast}2 always fails",
(Type, Type, unsigned))
NOTE(checked_cast_not_supported_coerce_instead,none,
"consider using 'as' coercion instead", ())

WARNING(conditional_downcast_coercion,none,
"conditional cast from %0 to %1 always succeeds",
(Type, Type))
Expand Down
3 changes: 3 additions & 0 deletions include/swift/AST/Types.h
Original file line number Diff line number Diff line change
Expand Up @@ -3139,6 +3139,9 @@ class AnyFunctionType : public TypeBase {

AnyFunctionType *getWithoutDifferentiability() const;

/// Return the function type without the throwing.
AnyFunctionType *getWithoutThrowing() const;

/// True if the parameter declaration it is attached to is guaranteed
/// to not persist the closure for longer than the duration of the call.
bool isNoEscape() const {
Expand Down
87 changes: 87 additions & 0 deletions include/swift/Sema/CSFix.h
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,19 @@ enum class FixKind : uint8_t {
/// Explicitly specify the type to disambiguate between possible member base
/// types.
SpecifyBaseTypeForOptionalUnresolvedMember,

/// Allow a runtime checked cast from an optional type where we statically
/// know the result is always succeed.
AllowCheckedCastCoercibleOptionalType,

/// Allow a runtime checked cast where we statically know the result
/// is always succeed.
AllowAlwaysSucceedCheckedCast,

/// Allow a runtime checked cast where at compile time the from is
/// convertible, but runtime does not support such convertions. e.g.
/// function type casts.
AllowUnsupportedRuntimeCheckedCast,
};

class ConstraintFix {
Expand Down Expand Up @@ -2196,6 +2209,80 @@ class SpecifyBaseTypeForOptionalUnresolvedMember final : public ConstraintFix {
MemberLookupResult result, ConstraintLocator *locator);
};

class CheckedCastContextualMismatchWarning : public ContextualMismatch {
protected:
CheckedCastContextualMismatchWarning(ConstraintSystem &cs, FixKind fixKind,
Type fromType, Type toType,
CheckedCastKind kind,
ConstraintLocator *locator)
: ContextualMismatch(cs, fixKind, fromType, toType, locator,
/*isWarning*/ true),
CastKind(kind) {}
CheckedCastKind CastKind;
};

class AllowCheckedCastCoercibleOptionalType final
: public CheckedCastContextualMismatchWarning {
AllowCheckedCastCoercibleOptionalType(ConstraintSystem &cs, Type fromType,
Type toType, CheckedCastKind kind,
ConstraintLocator *locator)
: CheckedCastContextualMismatchWarning(
cs, FixKind::AllowCheckedCastCoercibleOptionalType, fromType,
toType, kind, locator) {}

public:
std::string getName() const override {
return "checked cast coercible optional";
}
bool diagnose(const Solution &solution, bool asNote = false) const override;

static AllowCheckedCastCoercibleOptionalType *
create(ConstraintSystem &cs, Type fromType, Type toType, CheckedCastKind kind,
ConstraintLocator *locator);
};

class AllowAlwaysSucceedCheckedCast final
: public CheckedCastContextualMismatchWarning {
AllowAlwaysSucceedCheckedCast(ConstraintSystem &cs, Type fromType,
Type toType, CheckedCastKind kind,
ConstraintLocator *locator)
: CheckedCastContextualMismatchWarning(
cs, FixKind::AllowUnsupportedRuntimeCheckedCast, fromType, toType,
kind, locator) {}

public:
std::string getName() const override { return "checked cast always succeed"; }
bool diagnose(const Solution &solution, bool asNote = false) const override;

static AllowAlwaysSucceedCheckedCast *create(ConstraintSystem &cs,
Type fromType, Type toType,
CheckedCastKind kind,
ConstraintLocator *locator);
};

class AllowUnsupportedRuntimeCheckedCast final
: public CheckedCastContextualMismatchWarning {
AllowUnsupportedRuntimeCheckedCast(ConstraintSystem &cs, Type fromType,
Type toType, CheckedCastKind kind,
ConstraintLocator *locator)
: CheckedCastContextualMismatchWarning(
cs, FixKind::AllowUnsupportedRuntimeCheckedCast, fromType, toType,
kind, locator) {}

public:
std::string getName() const override {
return "runtime unsupported checked cast";
}
bool diagnose(const Solution &solution, bool asNote = false) const override;

static bool runtimeSupportedFunctionTypeCast(FunctionType *fnFromType,
FunctionType *fnToType);

static AllowUnsupportedRuntimeCheckedCast *
attempt(ConstraintSystem &cs, Type fromType, Type toType,
CheckedCastKind kind, ConstraintLocator *locator);
};

} // end namespace constraints
} // end namespace swift

Expand Down
5 changes: 5 additions & 0 deletions lib/AST/Type.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5115,6 +5115,11 @@ AnyFunctionType *AnyFunctionType::getWithoutDifferentiability() const {
getResult(), nonDiffExtInfo);
}

AnyFunctionType *AnyFunctionType::getWithoutThrowing() const {
auto info = getExtInfo().intoBuilder().withThrows(false).build();
return withExtInfo(info);
}

Optional<TangentSpace>
TypeBase::getAutoDiffTangentSpace(LookupConformanceFn lookupConformance) {
assert(lookupConformance);
Expand Down
18 changes: 0 additions & 18 deletions lib/Sema/CSApply.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3558,8 +3558,6 @@ namespace {

case CheckedCastKind::Coercion:
case CheckedCastKind::BridgingCoercion:
// Check is trivially true.
ctx.Diags.diagnose(expr->getLoc(), diag::isa_is_always_true, "is");
expr->setCastKind(castKind);
break;
case CheckedCastKind::ValueCast:
Expand Down Expand Up @@ -3979,7 +3977,6 @@ namespace {
// Rewrite ForcedCheckedCastExpr based on what the solver computed.
Expr *visitForcedCheckedCastExpr(ForcedCheckedCastExpr *expr) {
// The subexpression is always an rvalue.
auto &ctx = cs.getASTContext();
auto sub = cs.coerceToRValue(expr->getSubExpr());
expr->setSubExpr(sub);

Expand Down Expand Up @@ -4007,19 +4004,6 @@ namespace {
return nullptr;
case CheckedCastKind::Coercion:
case CheckedCastKind::BridgingCoercion: {
if (cs.getType(sub)->isEqual(toType)) {
ctx.Diags.diagnose(expr->getLoc(), diag::forced_downcast_noop, toType)
.fixItRemove(SourceRange(expr->getLoc(),
castTypeRepr->getSourceRange().End));

} else {
ctx.Diags
.diagnose(expr->getLoc(), diag::forced_downcast_coercion,
cs.getType(sub), toType)
.fixItReplace(SourceRange(expr->getLoc(), expr->getExclaimLoc()),
"as");
}

expr->setCastKind(castKind);
cs.setType(expr, toType);
return expr;
Expand Down Expand Up @@ -4111,8 +4095,6 @@ namespace {

case CheckedCastKind::Coercion:
case CheckedCastKind::BridgingCoercion: {
ctx.Diags.diagnose(expr->getLoc(), diag::conditional_downcast_coercion,
fromType, toType);
expr->setCastKind(castKind);
cs.setType(expr, OptionalType::get(toType));
return expr;
Expand Down
Loading