Skip to content

Commit

Permalink
feat: substitute type parameter when type checking calls (#923)
Browse files Browse the repository at this point in the history 
Closes #915

### Summary of Changes

* Substitute type parameter when type checking calls.
* Remove lenient type checking mode. We now use the strict mode
everywhere, which means when calling `isSubtypeOf(type, other)` that the
upper bound of `type` must be a subtype of the lower bound of `other`.
  • Loading branch information
@lars-reimann
lars-reimann authored Feb 26, 2024
1 parent 0e657cf commit 2e09306
Show file tree
Hide file tree
Showing 15 changed files with 269 additions and 111 deletions.
4 changes: 1 addition & 3 deletions packages/safe-ds-lang/src/language/typing/model.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -737,9 +737,7 @@ export class UnionType extends Type {
const candidateType = otherType.withExplicitNullability(
currentType.isExplicitlyNullable || otherType.isExplicitlyNullable,
);
if (
this.typeChecker.isSupertypeOf(candidateType, currentType, { strictTypeParameterTypeCheck: true })
) {
if (this.typeChecker.isSupertypeOf(candidateType, currentType)) {
// Replace the other type with the candidate type (updated nullability)
newTypes.splice(j, 1, candidateType);
// Remove the current type
Expand Down
61 changes: 25 additions & 36 deletions packages/safe-ds-lang/src/language/typing/safe-ds-type-checker.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,22 +57,17 @@ export class SafeDsTypeChecker {
}

if (other instanceof TypeParameterType) {
if (options.strictTypeParameterTypeCheck) {
// `T` can always be assigned to `T` and `T?`
if (
type instanceof TypeParameterType &&
type.declaration === other.declaration &&
(!type.isExplicitlyNullable || other.isExplicitlyNullable)
) {
return true;
}
if (type.isExplicitlyNullable && !other.isExplicitlyNullable) {
return false;
}

const otherLowerBound = this.coreTypes.Nothing.withExplicitNullability(other.isExplicitlyNullable);
return this.isSubtypeOf(type, otherLowerBound, options);
} else {
const otherUpperBound = this.typeComputer().computeUpperBound(other);
return this.isSubtypeOf(type, otherUpperBound, options);
// `T` can always be assigned to `T` or some type parameter it is bounded by
if (type instanceof TypeParameterType && this.typeParameterIsBoundedByTypeParameter(type, other)) {
return true;
}

const otherLowerBound = this.coreTypes.Nothing.withExplicitNullability(other.isExplicitlyNullable);
return this.isSubtypeOf(type, otherLowerBound, options);
} else if (other instanceof UnionType) {
return other.types.some((it) => this.isSubtypeOf(type, it, options));
}
Expand Down Expand Up @@ -100,6 +95,20 @@ export class SafeDsTypeChecker {
} /* c8 ignore stop */
};

private typeParameterIsBoundedByTypeParameter(type: TypeParameterType, other: TypeParameterType): boolean {
let current: Type = type;

while (current instanceof TypeParameterType) {
if (current.declaration === other.declaration) {
return true;
}

current = this.typeComputer().computeUpperBound(current, { stopAtTypeParameterType: true });
}

return false;
}

private callableTypeIsSubtypeOf(type: CallableType, other: Type, options: TypeCheckOptions): boolean {
if (other instanceof ClassType) {
return other.declaration === this.builtinClasses.Any;
Expand Down Expand Up @@ -299,14 +308,8 @@ export class SafeDsTypeChecker {
}

private typeParameterTypeIsSubtypeOf(type: TypeParameterType, other: Type, options: TypeCheckOptions): boolean {
if (options.strictTypeParameterTypeCheck) {
const upperBound = this.typeComputer().computeUpperBound(type);
return this.isSubtypeOf(upperBound, other, options);
} else {
// We need to check whether `type` is assignable to `other` after substituting `type` with its lower bound.
// Since this bound is `Nothing`, which is a subtype of everything, it boils down to a nullability check.
return !type.isExplicitlyNullable || other.isExplicitlyNullable;
}
const upperBound = this.typeComputer().computeUpperBound(type);
return this.isSubtypeOf(upperBound, other, options);
}

private unionTypeIsSubtypeOf(type: UnionType, other: Type, options: TypeCheckOptions): boolean {
Expand Down Expand Up @@ -395,7 +398,6 @@ export class SafeDsTypeChecker {
!type.equals(this.coreTypes.NothingOrNull) &&
this.isSubtypeOf(type, listOrNull, {
ignoreTypeParameters: true,
strictTypeParameterTypeCheck: true,
})
);
}
Expand All @@ -411,7 +413,6 @@ export class SafeDsTypeChecker {
!type.equals(this.coreTypes.NothingOrNull) &&
this.isSubtypeOf(type, mapOrNull, {
ignoreTypeParameters: true,
strictTypeParameterTypeCheck: true,
})
);
}
Expand All @@ -425,16 +426,4 @@ interface TypeCheckOptions {
* Whether to ignore type parameters when comparing class types.
*/
ignoreTypeParameters?: boolean;

/**
* By default, type parameter types are replaced with their upper bound when comparing types. This is usually the
* correct behavior, e.g. to check whether the type `Int` can be assigned to an unsubstituted type parameter type
* `T`.
*
* However, in some cases, we have to assume that the type parameter type that we compare to gets substituted later,
* e.g. by its lower bound `Nothing`. This options enables a strict check, replacing type parameter types in the
* first argument of {@link SafeDsTypeChecker.isSubtypeOf} with their upper bound, and in the second argument with
* their lower bound.
*/
strictTypeParameterTypeCheck?: boolean;
}
39 changes: 18 additions & 21 deletions packages/safe-ds-lang/src/language/typing/safe-ds-type-computer.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -348,9 +348,7 @@ export class SafeDsTypeComputer {
// as `Literal<"a", "b">`. But then we would be unable to pass an unknown `String` as the key in an indexed
// access. Where possible, we already validate the existence of keys in indexed accesses using the partial
// evaluator.
if (keyType instanceof LiteralType) {
keyType = this.computeClassTypeForLiteralType(keyType);
}
keyType = this.computeClassTypeForLiteralType(keyType);

const valueType = this.lowestCommonSupertype(node.entries.map((it) => this.computeType(it.value)));
return this.coreTypes.Map(keyType, valueType);
Expand Down Expand Up @@ -457,7 +455,7 @@ export class SafeDsTypeComputer {

// Substitute type parameters
if (isSdsFunction(nonNullableReceiverType.callable)) {
const substitutions = this.computeTypeParameterSubstitutionsForCall(node);
const substitutions = this.computeSubstitutionsForCall(node);
result = result.substituteTypeParameters(substitutions);
}
} else if (nonNullableReceiverType instanceof StaticType) {
Expand All @@ -468,7 +466,7 @@ export class SafeDsTypeComputer {

// Substitute type parameters
if (instanceType instanceof ClassType) {
const substitutions = this.computeTypeParameterSubstitutionsForCall(node);
const substitutions = this.computeSubstitutionsForCall(node);

result = this.factory.createClassType(
instanceType.declaration,
Expand Down Expand Up @@ -528,8 +526,8 @@ export class SafeDsTypeComputer {
const rightOperandType = this.computeType(node.rightOperand);

if (
this.typeChecker.isSubtypeOf(leftOperandType, this.coreTypes.Int, { strictTypeParameterTypeCheck: true }) &&
this.typeChecker.isSubtypeOf(rightOperandType, this.coreTypes.Int, { strictTypeParameterTypeCheck: true })
this.typeChecker.isSubtypeOf(leftOperandType, this.coreTypes.Int) &&
this.typeChecker.isSubtypeOf(rightOperandType, this.coreTypes.Int)
) {
return this.coreTypes.Int;
} else {
Expand Down Expand Up @@ -581,7 +579,7 @@ export class SafeDsTypeComputer {
private computeTypeOfArithmeticPrefixOperation(node: SdsPrefixOperation): Type {
const operandType = this.computeType(node.operand);

if (this.typeChecker.isSubtypeOf(operandType, this.coreTypes.Int, { strictTypeParameterTypeCheck: true })) {
if (this.typeChecker.isSubtypeOf(operandType, this.coreTypes.Int)) {
return this.coreTypes.Int;
} else {
return this.coreTypes.Float;
Expand Down Expand Up @@ -679,10 +677,15 @@ export class SafeDsTypeComputer {
}

/**
* Returns the lowest class type for the given literal type.
* Returns the lowest class type for the given literal type. If the given type is not a literal type, it is returned
* as is.
*/
computeClassTypeForLiteralType(literalType: LiteralType): Type {
return this.lowestCommonSupertype(literalType.constants.map((it) => this.computeClassTypeForConstant(it)));
computeClassTypeForLiteralType(type: Type): Type {
if (!(type instanceof LiteralType)) {
return type;
}

return this.lowestCommonSupertype(type.constants.map((it) => this.computeClassTypeForConstant(it)));
}

/**
Expand Down Expand Up @@ -783,7 +786,7 @@ export class SafeDsTypeComputer {
* @param call The call to compute substitutions for.
* @returns The computed substitutions for the type parameters of the callable.
*/
computeTypeParameterSubstitutionsForCall(call: SdsCall): TypeParameterSubstitutions {
computeSubstitutionsForCall(call: SdsCall): TypeParameterSubstitutions {
const callable = this.nodeMapper.callToCallable(call);
const typeParameters = getTypeParameters(callable);
if (isEmpty(typeParameters)) {
Expand Down Expand Up @@ -874,7 +877,7 @@ export class SafeDsTypeComputer {
stopAtTypeParameterType: true,
}).substituteTypeParameters(state.substitutions);

if (!this.typeChecker.isSubtypeOf(newSubstitution, upperBound, { strictTypeParameterTypeCheck: true })) {
if (!this.typeChecker.isSubtypeOf(newSubstitution, upperBound)) {
newSubstitution = upperBound;
}

Expand Down Expand Up @@ -1243,15 +1246,12 @@ export class SafeDsTypeComputer {
return otherTypes.every((it) =>
this.typeChecker.isSupertypeOf(candidate, it, {
ignoreTypeParameters: true,
strictTypeParameterTypeCheck: true,
}),
);
}

private isCommonSupertype(candidate: Type, otherTypes: Type[]): boolean {
return otherTypes.every((it) =>
this.typeChecker.isSupertypeOf(candidate, it, { strictTypeParameterTypeCheck: true }),
);
return otherTypes.every((it) => this.typeChecker.isSupertypeOf(candidate, it));
}

// -----------------------------------------------------------------------------------------------------------------
Expand Down Expand Up @@ -1486,15 +1486,12 @@ export class SafeDsTypeComputer {
return otherTypes.every((it) =>
this.typeChecker.isSubtypeOf(candidate, it, {
ignoreTypeParameters: true,
strictTypeParameterTypeCheck: true,
}),
);
}

private isCommonSubtype(candidate: Type, otherTypes: Type[]): boolean {
return otherTypes.every((it) =>
this.typeChecker.isSubtypeOf(candidate, it, { strictTypeParameterTypeCheck: true }),
);
return otherTypes.every((it) => this.typeChecker.isSubtypeOf(candidate, it));
}

// -----------------------------------------------------------------------------------------------------------------
Expand Down
12 changes: 2 additions & 10 deletions packages/safe-ds-lang/src/language/validation/inheritance.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,7 @@ export const classMemberMustMatchOverriddenMemberAndShouldBeNeeded = (services:
computeMemberTypes(node, overriddenMember, typeComputer);

// Check whether the overriding is legal and needed
if (
!typeChecker.isSubtypeOf(substitutedOwnMemberType, overriddenMemberType, {
strictTypeParameterTypeCheck: true,
})
) {
if (!typeChecker.isSubtypeOf(substitutedOwnMemberType, overriddenMemberType)) {
accept(
'error',
expandToStringWithNL`
Expand All @@ -49,11 +45,7 @@ export const classMemberMustMatchOverriddenMemberAndShouldBeNeeded = (services:
code: CODE_INHERITANCE_INCOMPATIBLE_TO_OVERRIDDEN_MEMBER,
},
);
} else if (
typeChecker.isSubtypeOf(substitutedOverriddenMemberType, ownMemberType, {
strictTypeParameterTypeCheck: true,
})
) {
} else if (typeChecker.isSubtypeOf(substitutedOverriddenMemberType, ownMemberType)) {
// Prevents the info from showing when editing the builtin files
if (isInSafedsLangAnyClass(services, node)) {
return;
Expand Down
4 changes: 2 additions & 2 deletions packages/safe-ds-lang/src/language/validation/safe-ds-validator.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -157,8 +157,8 @@ import {
unionTypeShouldNotHaveASingularTypeArgument,
} from './style.js';
import {
argumentTypeMustMatchParameterType,
attributeMustHaveTypeHint,
callArgumentTypesMustMatchParameterTypes,
callReceiverMustBeCallable,
indexedAccessIndexMustHaveCorrectType,
indexedAccessReceiverMustBeListOrMap,
Expand Down Expand Up @@ -216,7 +216,6 @@ export const registerValidationChecks = function (services: SafeDsServices) {
SdsArgument: [
argumentCorrespondingParameterShouldNotBeDeprecated(services),
argumentCorrespondingParameterShouldNotBeExperimental(services),
argumentTypeMustMatchParameterType(services),
],
SdsArgumentList: [
argumentListMustNotHavePositionalArgumentsAfterNamedArguments,
Expand All @@ -228,6 +227,7 @@ export const registerValidationChecks = function (services: SafeDsServices) {
callArgumentListShouldBeNeeded(services),
callArgumentAssignedToPureParameterMustBePure(services),
callArgumentMustBeConstantIfParameterIsConstant(services),
callArgumentTypesMustMatchParameterTypes(services),
callMustNotBeRecursive(services),
callReceiverMustBeCallable(services),
],
Expand Down
39 changes: 21 additions & 18 deletions packages/safe-ds-lang/src/language/validation/types.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import {
isSdsPipeline,
isSdsReference,
isSdsSchema,
SdsArgument,
SdsAttribute,
SdsCall,
SdsIndexedAccess,
Expand All @@ -25,7 +24,7 @@ import {
SdsTypeParameter,
SdsYield,
} from '../generated/ast.js';
import { getTypeArguments, getTypeParameters, TypeParameter } from '../helpers/nodeProperties.js';
import { getArguments, getTypeArguments, getTypeParameters, TypeParameter } from '../helpers/nodeProperties.js';
import { SafeDsServices } from '../safe-ds-module.js';
import { ClassType, NamedTupleType, TypeParameterType, UnknownType } from '../typing/model.js';

Expand All @@ -38,26 +37,30 @@ export const CODE_TYPE_MISSING_TYPE_HINT = 'type/missing-type-hint';
// Type checking
// -----------------------------------------------------------------------------

export const argumentTypeMustMatchParameterType = (services: SafeDsServices) => {
export const callArgumentTypesMustMatchParameterTypes = (services: SafeDsServices) => {
const nodeMapper = services.helpers.NodeMapper;
const typeChecker = services.types.TypeChecker;
const typeComputer = services.types.TypeComputer;

return (node: SdsArgument, accept: ValidationAcceptor) => {
const parameter = nodeMapper.argumentToParameter(node);
if (!parameter) {
return;
}
return (node: SdsCall, accept: ValidationAcceptor) => {
const substitutions = typeComputer.computeSubstitutionsForCall(node);

const argumentType = typeComputer.computeType(node);
const parameterType = typeComputer.computeType(parameter);
for (const argument of getArguments(node)) {
const parameter = nodeMapper.argumentToParameter(argument);
if (!parameter) {
return;
}

if (!typeChecker.isSubtypeOf(argumentType, parameterType)) {
accept('error', `Expected type '${parameterType}' but got '${argumentType}'.`, {
node,
property: 'value',
code: CODE_TYPE_MISMATCH,
});
const argumentType = typeComputer.computeType(argument).substituteTypeParameters(substitutions);
const parameterType = typeComputer.computeType(parameter).substituteTypeParameters(substitutions);

if (!typeChecker.isSubtypeOf(argumentType, parameterType)) {
accept('error', `Expected type '${parameterType}' but got '${argumentType}'.`, {
node: argument,
property: 'value',
code: CODE_TYPE_MISMATCH,
});
}
}
};
};
Expand Down Expand Up @@ -279,7 +282,7 @@ export const namedTypeTypeArgumentsMustMatchBounds = (services: SafeDsServices)
.computeUpperBound(typeParameter, { stopAtTypeParameterType: true })
.substituteTypeParameters(type.substitutions);

if (!typeChecker.isSubtypeOf(typeArgumentType, upperBound, { strictTypeParameterTypeCheck: true })) {
if (!typeChecker.isSubtypeOf(typeArgumentType, upperBound)) {
accept('error', `Expected type '${upperBound}' but got '${typeArgumentType}'.`, {
node: typeArgument,
property: 'value',
Expand Down Expand Up @@ -377,7 +380,7 @@ export const typeParameterDefaultValueMustMatchUpperBound = (services: SafeDsSer
const defaultValueType = typeComputer.computeType(node.defaultValue);
const upperBoundType = typeComputer.computeUpperBound(node, { stopAtTypeParameterType: true });

if (!typeChecker.isSubtypeOf(defaultValueType, upperBoundType, { strictTypeParameterTypeCheck: true })) {
if (!typeChecker.isSubtypeOf(defaultValueType, upperBoundType)) {
accept('error', `Expected type '${upperBoundType}' but got '${defaultValueType}'.`, {
node,
property: 'defaultValue',
Expand Down
Loading

0 comments on commit 2e09306

Please sign in to comment.