Skip to content

[Type checker] Clean up expression type checking flags #29589

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
Feb 1, 2020
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
9 changes: 2 additions & 7 deletions lib/Sema/CSDiag.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -394,15 +394,10 @@ Expr *FailureDiagnosis::typeCheckChildIndependently(
// type check operation.
Expr *preCheckedExpr = subExpr;

// Disable structural checks, because we know that the overall expression
// has type constraint problems, and we don't want to know about any
// syntactic issues in a well-typed subexpression (which might be because
// the context is missing).
TypeCheckExprOptions TCEOptions = TypeCheckExprFlags::DisableStructuralChecks;

// Make sure that typechecker knows that this is an attempt
// to diagnose a problem.
TCEOptions |= TypeCheckExprFlags::SubExpressionDiagnostics;
TypeCheckExprOptions TCEOptions =
TypeCheckExprFlags::SubExpressionDiagnostics;

// Claim that the result is discarded to preserve the lvalue type of
// the expression.
Expand Down
2 changes: 1 addition & 1 deletion lib/Sema/CSDiagnostics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -641,7 +641,7 @@ bool GenericArgumentsMismatchFailure::diagnoseAsError() {
switch (last.getKind()) {
case ConstraintLocator::ContextualType: {
auto purpose = getContextualTypePurpose();
assert(!(purpose == CTP_Unused && purpose == CTP_CannotFail));
assert(!(purpose == CTP_Unused || purpose == CTP_CannotFail));
Copy link
Contributor

Choose a reason for hiding this comment

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

Oops...


// If this is call to a closure e.g. `let _: A = { B() }()`
// let's point diagnostic to its result.
Expand Down
35 changes: 31 additions & 4 deletions lib/Sema/ConstraintSystem.h
Original file line number Diff line number Diff line change
Expand Up @@ -789,7 +789,7 @@ using OpenedTypeMap =

/// Describes contextual type information about a particular expression
/// within a constraint system.
struct ContextualTypeInfo {
struct ContextualTypeInfo {
TypeLoc typeLoc;
ContextualTypePurpose purpose;
bool isOpaqueReturnType = false;
Expand Down Expand Up @@ -1149,8 +1149,11 @@ class SolutionApplicationTarget {
struct {
Expr *expression;

/// The purpose of the contextual type.
ContextualTypePurpose contextualPurpose;

/// The type to which the expression should be converted.
Type convertType;
TypeLoc convertType;

/// Whether the expression result will be discarded at the end.
bool isDiscarded;
Expand All @@ -1163,9 +1166,18 @@ class SolutionApplicationTarget {
};

public:
SolutionApplicationTarget(Expr *expr, Type convertType, bool isDiscarded) {
SolutionApplicationTarget(Expr *expr,
ContextualTypePurpose contextualPurpose,
Type convertType, bool isDiscarded)
: SolutionApplicationTarget(expr, contextualPurpose,
TypeLoc::withoutLoc(convertType),
isDiscarded) { }

SolutionApplicationTarget(Expr *expr, ContextualTypePurpose contextualPurpose,
TypeLoc convertType, bool isDiscarded) {
kind = Kind::expression;
expression.expression = expr;
expression.contextualPurpose = contextualPurpose;
expression.convertType = convertType;
expression.isDiscarded = isDiscarded;
}
Expand All @@ -1189,16 +1201,31 @@ class SolutionApplicationTarget {
}
}

ContextualTypePurpose getExprContextualTypePurpose() const {
assert(kind == Kind::expression);
return expression.contextualPurpose;
}

Type getExprConversionType() const {
assert(kind == Kind::expression);
return expression.convertType.getType();
}

TypeLoc getExprConversionTypeLoc() const {
assert(kind == Kind::expression);
return expression.convertType;
}

void setExprConversionType(Type type) {
assert(kind == Kind::expression);
expression.convertType = TypeLoc::withoutLoc(type);
}

void setExprConversionTypeLoc(TypeLoc type) {
assert(kind == Kind::expression);
expression.convertType = type;
}

bool isDiscardedExpr() const {
assert(kind == Kind::expression);
return expression.isDiscarded;
Expand Down
51 changes: 43 additions & 8 deletions lib/Sema/TypeCheckConstraints.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2058,6 +2058,40 @@ bool GenericRequirementsCheckListener::diagnoseUnsatisfiedRequirement(
return false;
}

/// Whether the contextual type provided for the given purpose is only a
/// hint, and not a requirement.
static bool contextualTypeIsOnlyAHint(ContextualTypePurpose ctp,
TypeCheckExprOptions options) {
switch (ctp) {
case CTP_Initialization:
return !options.contains(
TypeCheckExprFlags::ConvertTypeIsOpaqueReturnType);
case CTP_ForEachStmt:
return true;
case CTP_Unused:
case CTP_ReturnStmt:
case CTP_ReturnSingleExpr:
case CTP_YieldByValue:
case CTP_YieldByReference:
case CTP_ThrowStmt:
case CTP_EnumCaseRawValue:
case CTP_DefaultParameter:
case CTP_AutoclosureDefaultParameter:
case CTP_CalleeResult:
case CTP_CallArgument:
case CTP_ClosureResult:
case CTP_ArrayElement:
case CTP_DictionaryKey:
case CTP_DictionaryValue:
case CTP_CoerceOperand:
case CTP_AssignSource:
case CTP_SubscriptAssignSource:
case CTP_Condition:
case CTP_CannotFail:
return false;
}
}

#pragma mark High-level entry points
Type TypeChecker::typeCheckExpression(Expr *&expr, DeclContext *dc,
TypeLoc convertType,
Expand Down Expand Up @@ -2128,7 +2162,7 @@ Type TypeChecker::typeCheckExpression(Expr *&expr, DeclContext *dc,

// If the convertType is *only* provided for that hint, then null it out so
// that we don't later treat it as an actual conversion constraint.
if (options.contains(TypeCheckExprFlags::ConvertTypeIsOnlyAHint))
if (contextualTypeIsOnlyAHint(convertTypePurpose, options))
convertType = TypeLoc();

// If the client can handle unresolved type variables, leave them in the
Expand All @@ -2149,7 +2183,7 @@ Type TypeChecker::typeCheckExpression(Expr *&expr, DeclContext *dc,

// Attempt to solve the constraint system.
SolutionApplicationTarget target(
expr, convertTo,
expr, convertTypePurpose, convertTo,
options.contains(TypeCheckExprFlags::IsDiscarded));
auto viable = cs.solve(target, listener, allowFreeTypeVariables);
if (!viable)
Expand Down Expand Up @@ -2206,7 +2240,7 @@ Type TypeChecker::typeCheckExpression(Expr *&expr, DeclContext *dc,
// Unless the client has disabled them, perform syntactic checks on the
// expression now.
if (!cs.shouldSuppressDiagnostics() &&
!options.contains(TypeCheckExprFlags::DisableStructuralChecks)) {
!options.contains(TypeCheckExprFlags::SubExpressionDiagnostics)) {
bool isExprStmt = options.contains(TypeCheckExprFlags::IsExprStmt);
performSyntacticExprDiagnostics(result, dc, isExprStmt);
}
Expand Down Expand Up @@ -2249,7 +2283,8 @@ getTypeOfExpressionWithoutApplying(Expr *&expr, DeclContext *dc,
// re-check.
if (needClearType)
expr->setType(Type());
SolutionApplicationTarget target(expr, Type(), /*isDiscarded=*/false);
SolutionApplicationTarget target(
expr, CTP_Unused, Type(), /*isDiscarded=*/false);
auto viable = cs.solve(target, listener, allowFreeTypeVariables);
if (!viable) {
recoverOriginalType();
Expand Down Expand Up @@ -2329,7 +2364,8 @@ void TypeChecker::getPossibleTypesOfExpressionWithoutApplying(
if (originalType && originalType->hasError())
expr->setType(Type());

SolutionApplicationTarget target(expr, Type(), /*isDiscarded=*/false);
SolutionApplicationTarget target(
expr, CTP_Unused, Type(), /*isDiscarded=*/false);
if (auto viable = cs.solve(target, listener, allowFreeTypeVariables)) {
expr = target.getAsExpr();
for (auto &solution : *viable) {
Expand Down Expand Up @@ -2655,7 +2691,7 @@ bool TypeChecker::typeCheckBinding(Pattern *&pattern, Expr *&initializer,

TypeLoc contextualType;
auto contextualPurpose = CTP_Unused;
TypeCheckExprOptions flags = TypeCheckExprFlags::ConvertTypeIsOnlyAHint;
TypeCheckExprOptions flags = None;

// Set the contextual purpose even if the pattern doesn't have a type so
// if there's an error we can use that information to inform diagnostics.
Expand All @@ -2674,7 +2710,6 @@ bool TypeChecker::typeCheckBinding(Pattern *&pattern, Expr *&initializer,
// opaque type.
if (auto opaqueType = patternType->getAs<OpaqueTypeArchetypeType>()){
flags |= TypeCheckExprFlags::ConvertTypeIsOpaqueReturnType;
flags -= TypeCheckExprFlags::ConvertTypeIsOnlyAHint;
}

// Only provide a TypeLoc if it makes sense to allow diagnostics.
Expand Down Expand Up @@ -3042,7 +3077,7 @@ bool TypeChecker::typeCheckForEachBinding(DeclContext *dc, ForEachStmt *stmt) {
// Type-check the for-each loop sequence and element pattern.
auto resultTy = TypeChecker::typeCheckExpression(
seq, dc, TypeLoc::withoutLoc(sequenceProto->getDeclaredType()),
CTP_ForEachStmt, TypeCheckExprFlags::ConvertTypeIsOnlyAHint, &listener);
CTP_ForEachStmt, None, &listener);
if (!resultTy)
return true;
return false;
Expand Down
27 changes: 3 additions & 24 deletions lib/Sema/TypeChecker.h
Original file line number Diff line number Diff line change
Expand Up @@ -163,31 +163,18 @@ enum class TypeCheckExprFlags {
/// disables constraints forcing an lvalue result to be loadable.
IsDiscarded = 0x01,

/// Whether the client wants to disable the structural syntactic restrictions
/// that we force for style or other reasons.
DisableStructuralChecks = 0x02,

/// If set, the client wants a best-effort solution to the constraint system,
/// but can tolerate a solution where all of the constraints are solved, but
/// not all type variables have been determined. In this case, the constraint
/// system is not applied to the expression AST, but the ConstraintSystem is
/// left in-tact.
AllowUnresolvedTypeVariables = 0x08,

/// If set, the 'convertType' specified to typeCheckExpression should not
/// produce a conversion constraint, but it should be used to guide the
/// solution in terms of performance optimizations of the solver, and in terms
/// of guiding diagnostics.
ConvertTypeIsOnlyAHint = 0x10,

/// If set, this expression isn't embedded in a larger expression or
/// statement. This should only be used for syntactic restrictions, and should
/// not affect type checking itself.
IsExprStmt = 0x20,

/// This is an inout yield.
IsInOutYield = 0x100,

/// If set, a conversion constraint should be specified so that the result of
/// the expression is an optional type.
ExpressionTypeMustBeOptional = 0x200,
Expand Down Expand Up @@ -821,11 +808,9 @@ class TypeChecker final {
/// to be possible.
///
/// \param convertType The type that the expression is being converted to,
/// or null if the expression is standalone. If the 'ConvertTypeIsOnlyAHint'
/// option is specified, then this is only a hint, it doesn't produce a full
/// conversion constraint. The location information is only used for
/// diagnostics should the conversion fail; it is safe to pass a TypeLoc
/// without location information.
/// or null if the expression is standalone. The location information is
/// only used for diagnostics should the conversion fail; it is safe to pass
/// a TypeLoc without location information.
///
/// \param options Options that control how type checking is performed.
///
Expand All @@ -847,12 +832,6 @@ class TypeChecker final {
ExprTypeCheckListener *listener = nullptr,
constraints::ConstraintSystem *baseCS = nullptr);

static Type typeCheckExpression(Expr *&expr, DeclContext *dc,
ExprTypeCheckListener *listener) {
return TypeChecker::typeCheckExpression(expr, dc, TypeLoc(), CTP_Unused,
TypeCheckExprOptions(), listener);
}

/// Type check the given expression and return its type without
/// applying the solution.
///
Expand Down