Skip to content

Commit

Permalink
feat: substitute type parameters when checking overridden members (#922)
Browse files Browse the repository at this point in the history 
Closes #917

### Summary of Changes

When checking whether overriding is legal and needed, we now use strict
type checking and correctly substitute type parameters.
  • Loading branch information
@lars-reimann
lars-reimann authored Feb 26, 2024
1 parent 76ad869 commit 0e657cf
Show file tree
Hide file tree
Showing 7 changed files with 220 additions and 74 deletions.
5 changes: 2 additions & 3 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 @@ -21,20 +21,17 @@ import { SafeDsClassHierarchy } from './safe-ds-class-hierarchy.js';
import { SafeDsCoreTypes } from './safe-ds-core-types.js';
import type { SafeDsTypeComputer } from './safe-ds-type-computer.js';
import { isEmpty } from '../../helpers/collections.js';
import { SafeDsTypeFactory } from './safe-ds-type-factory.js';

export class SafeDsTypeChecker {
private readonly builtinClasses: SafeDsClasses;
private readonly classHierarchy: SafeDsClassHierarchy;
private readonly coreTypes: SafeDsCoreTypes;
private readonly factory: SafeDsTypeFactory;
private readonly typeComputer: () => SafeDsTypeComputer;

constructor(services: SafeDsServices) {
this.builtinClasses = services.builtins.Classes;
this.classHierarchy = services.types.ClassHierarchy;
this.coreTypes = services.types.CoreTypes;
this.factory = services.types.TypeFactory;
this.typeComputer = () => services.types.TypeComputer;
}

Expand Down Expand Up @@ -398,6 +395,7 @@ export class SafeDsTypeChecker {
!type.equals(this.coreTypes.NothingOrNull) &&
this.isSubtypeOf(type, listOrNull, {
ignoreTypeParameters: true,
strictTypeParameterTypeCheck: true,
})
);
}
Expand All @@ -413,6 +411,7 @@ export class SafeDsTypeChecker {
!type.equals(this.coreTypes.NothingOrNull) &&
this.isSubtypeOf(type, mapOrNull, {
ignoreTypeParameters: true,
strictTypeParameterTypeCheck: true,
})
);
}
Expand Down
82 changes: 58 additions & 24 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 @@ -55,7 +55,6 @@ import {
SdsAssignee,
type SdsBlockLambda,
SdsCall,
SdsCallable,
SdsCallableType,
SdsClass,
SdsDeclaration,
Expand Down Expand Up @@ -458,11 +457,7 @@ export class SafeDsTypeComputer {

// Substitute type parameters
if (isSdsFunction(nonNullableReceiverType.callable)) {
const substitutions = this.computeTypeParameterSubstitutionsForCallable(
nonNullableReceiverType.callable,
node,
);

const substitutions = this.computeTypeParameterSubstitutionsForCall(node);
result = result.substituteTypeParameters(substitutions);
}
} else if (nonNullableReceiverType instanceof StaticType) {
Expand All @@ -473,7 +468,7 @@ export class SafeDsTypeComputer {

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

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

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

if (this.typeChecker.isSubtypeOf(operandType, this.coreTypes.Int)) {
if (this.typeChecker.isSubtypeOf(operandType, this.coreTypes.Int, { strictTypeParameterTypeCheck: true })) {
return this.coreTypes.Int;
} else {
return this.coreTypes.Float;
Expand Down Expand Up @@ -783,13 +778,13 @@ export class SafeDsTypeComputer {
// -----------------------------------------------------------------------------------------------------------------

/**
* Computes substitutions for the type parameters of the given callable in the context of the given call.
* Computes substitutions for the type parameters of a callable in the context of a call.
*
* @param callable The callable with the type parameters to compute substitutions for.
* @param call The call to compute substitutions for.
* @returns The computed substitutions for the type parameters of the callable.
*/
computeTypeParameterSubstitutionsForCallable(callable: SdsCallable, call: SdsCall): TypeParameterSubstitutions {
computeTypeParameterSubstitutionsForCall(call: SdsCall): TypeParameterSubstitutions {
const callable = this.nodeMapper.callToCallable(call);
const typeParameters = getTypeParameters(callable);
if (isEmpty(typeParameters)) {
return NO_SUBSTITUTIONS;
Expand All @@ -804,7 +799,36 @@ export class SafeDsTypeComputer {
return [this.computeType(parameter.type), this.computeType(argument?.value ?? parameter.defaultValue)];
});

return this.computeTypeParameterSubstitutionsForParameters(typeParameters, parameterTypesToArgumentTypes);
return this.computeTypeParameterSubstitutionsForArguments(typeParameters, parameterTypesToArgumentTypes);
}

/**
* Computes substitutions for the type parameters of a callable in the context of overriding another callable.
*
* @param ownMemberType The type of the overriding callable.
* @param overriddenMemberType The type of the overridden callable.
*/
computeSubstitutionsForOverriding(ownMemberType: Type, overriddenMemberType: Type): TypeParameterSubstitutions {
if (!(ownMemberType instanceof CallableType) || !(overriddenMemberType instanceof CallableType)) {
return NO_SUBSTITUTIONS;
}

const ownTypeParameters = getTypeParameters(ownMemberType.callable);
if (isEmpty(ownTypeParameters)) {
return NO_SUBSTITUTIONS;
}

const ownParameterTypes = ownMemberType.inputType.entries.map((it) => it.type);
const overriddenParameterTypes = overriddenMemberType.inputType.entries.map((it) => it.type);

const minimumParameterCount = Math.min(ownParameterTypes.length, overriddenParameterTypes.length);
const ownTypesToOverriddenTypes: [Type, Type][] = [];

for (let i = 0; i < minimumParameterCount; i++) {
ownTypesToOverriddenTypes.push([ownParameterTypes[i]!, overriddenParameterTypes[i]!]);
}

return this.computeTypeParameterSubstitutionsForArguments(ownTypeParameters, ownTypesToOverriddenTypes);
}

/**
Expand All @@ -815,7 +839,7 @@ export class SafeDsTypeComputer {
* @param parameterTypesToArgumentTypes Pairs of parameter types and the corresponding argument types.
* @returns The computed substitutions for the type parameters in the parameter types.
*/
computeTypeParameterSubstitutionsForParameters(
private computeTypeParameterSubstitutionsForArguments(
typeParameters: SdsTypeParameter[],
parameterTypesToArgumentTypes: [Type, Type][],
): TypeParameterSubstitutions {
Expand Down Expand Up @@ -850,7 +874,7 @@ export class SafeDsTypeComputer {
stopAtTypeParameterType: true,
}).substituteTypeParameters(state.substitutions);

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

Expand Down Expand Up @@ -1216,11 +1240,18 @@ export class SafeDsTypeComputer {
}

private isCommonSupertypeIgnoringTypeParameters(candidate: Type, otherTypes: Type[]): boolean {
return otherTypes.every((it) => this.typeChecker.isSupertypeOf(candidate, it, { ignoreTypeParameters: true }));
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));
return otherTypes.every((it) =>
this.typeChecker.isSupertypeOf(candidate, it, { strictTypeParameterTypeCheck: true }),
);
}

// -----------------------------------------------------------------------------------------------------------------
Expand All @@ -1245,9 +1276,7 @@ export class SafeDsTypeComputer {
const typesWithMatchingNullability = types.map((it) => it.withExplicitNullability(isNullable));

// One of the types is already a common subtype of all others after updating nullability
const commonSupertype = typesWithMatchingNullability.find((it) =>
this.isCommonSubtypeWithStrictTypeParameterTypeCheck(it, types),
);
const commonSupertype = typesWithMatchingNullability.find((it) => this.isCommonSubtype(it, types));
if (commonSupertype) {
return commonSupertype;
}
Expand Down Expand Up @@ -1318,7 +1347,7 @@ export class SafeDsTypeComputer {
// If the class has no type parameters, the candidate must match as is
const typeParameters = getTypeParameters(candidate.declaration);
if (isEmpty(typeParameters)) {
if (this.isCommonSubtypeWithStrictTypeParameterTypeCheck(candidate, others)) {
if (this.isCommonSubtype(candidate, others)) {
/* c8 ignore next 2 */
return candidate;
} else {
Expand Down Expand Up @@ -1454,10 +1483,15 @@ export class SafeDsTypeComputer {
}

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

private isCommonSubtypeWithStrictTypeParameterTypeCheck(candidate: Type, otherTypes: Type[]): boolean {
private isCommonSubtype(candidate: Type, otherTypes: Type[]): boolean {
return otherTypes.every((it) =>
this.typeChecker.isSubtypeOf(candidate, it, { strictTypeParameterTypeCheck: true }),
);
Expand Down
103 changes: 79 additions & 24 deletions packages/safe-ds-lang/src/language/validation/inheritance.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ import { isEmpty, isEqualSet } from '../../helpers/collections.js';
import { isSdsClass, isSdsFunction, SdsClass, type SdsClassMember } from '../generated/ast.js';
import { getParentTypes, getQualifiedName } from '../helpers/nodeProperties.js';
import { SafeDsServices } from '../safe-ds-module.js';
import { ClassType, UnknownType } from '../typing/model.js';
import { ClassType, Type, UnknownType } from '../typing/model.js';
import { SafeDsTypeComputer } from '../typing/safe-ds-type-computer.js';

export const CODE_INHERITANCE_CYCLE = 'inheritance/cycle';
export const CODE_INHERITANCE_MULTIPLE_INHERITANCE = 'inheritance/multiple-inheritance';
Expand All @@ -25,29 +26,16 @@ export const classMemberMustMatchOverriddenMemberAndShouldBeNeeded = (services:
return;
}

// Compute basic types (might contain type parameters)
const ownMemberType = typeComputer.computeType(node);
let overriddenMemberType = typeComputer.computeType(overriddenMember);
// Compute types
const { ownMemberType, overriddenMemberType, substitutedOwnMemberType, substitutedOverriddenMemberType } =
computeMemberTypes(node, overriddenMember, typeComputer);

// Substitute type parameters on overriddenMemberType
const classContainingOwnMember = getContainerOfType(node, isSdsClass);
const typeContainingOwnMember = typeComputer.computeType(classContainingOwnMember);

if (typeContainingOwnMember instanceof ClassType) {
const classContainingOverriddenMember = getContainerOfType(overriddenMember, isSdsClass);
const typeContainingOverriddenMember = typeComputer.computeMatchingSupertype(
typeContainingOwnMember,
classContainingOverriddenMember,
);

if (typeContainingOverriddenMember) {
overriddenMemberType = overriddenMemberType.substituteTypeParameters(
typeContainingOverriddenMember.substitutions,
);
}
}

if (!typeChecker.isSubtypeOf(ownMemberType, overriddenMemberType)) {
// Check whether the overriding is legal and needed
if (
!typeChecker.isSubtypeOf(substitutedOwnMemberType, overriddenMemberType, {
strictTypeParameterTypeCheck: true,
})
) {
accept(
'error',
expandToStringWithNL`
Expand All @@ -61,7 +49,11 @@ export const classMemberMustMatchOverriddenMemberAndShouldBeNeeded = (services:
code: CODE_INHERITANCE_INCOMPATIBLE_TO_OVERRIDDEN_MEMBER,
},
);
} else if (typeChecker.isSubtypeOf(overriddenMemberType, ownMemberType)) {
} else if (
typeChecker.isSubtypeOf(substitutedOverriddenMemberType, ownMemberType, {
strictTypeParameterTypeCheck: true,
})
) {
// Prevents the info from showing when editing the builtin files
if (isInSafedsLangAnyClass(services, node)) {
return;
Expand Down Expand Up @@ -94,6 +86,69 @@ export const classMemberMustMatchOverriddenMemberAndShouldBeNeeded = (services:
};
};

const computeMemberTypes = (
ownMember: SdsClassMember,
overriddenMember: SdsClassMember,
typeComputer: SafeDsTypeComputer,
): ComputeMemberTypesResult => {
// Compute basic types (might contain type parameters)
const ownMemberType = typeComputer.computeType(ownMember);
let overriddenMemberType = typeComputer.computeType(overriddenMember);

// Substitute type parameters of class containing the overridden member
const classContainingOwnMember = getContainerOfType(ownMember, isSdsClass);
const typeContainingOwnMember = typeComputer.computeType(classContainingOwnMember);

if (typeContainingOwnMember instanceof ClassType) {
const classContainingOverriddenMember = getContainerOfType(overriddenMember, isSdsClass);
const typeContainingOverriddenMember = typeComputer.computeMatchingSupertype(
typeContainingOwnMember,
classContainingOverriddenMember,
);

if (typeContainingOverriddenMember) {
overriddenMemberType = overriddenMemberType.substituteTypeParameters(
typeContainingOverriddenMember.substitutions,
);
}
}

// Substitute type parameters of methods
const substitutedOwnMemberType = ownMemberType.substituteTypeParameters(
typeComputer.computeSubstitutionsForOverriding(ownMemberType, overriddenMemberType),
);
const substitutedOverriddenMemberType = overriddenMemberType.substituteTypeParameters(
typeComputer.computeSubstitutionsForOverriding(overriddenMemberType, ownMemberType),
);

return { ownMemberType, overriddenMemberType, substitutedOwnMemberType, substitutedOverriddenMemberType };
};

interface ComputeMemberTypesResult {
/**
* The type of the own member. Type parameters of the containing class or own member are not yet substituted.
*/
ownMemberType: Type;

/**
* The type of the overridden member. Type parameters of the containing class are substituted, but not the type
* parameters of the overridden member.
*/
overriddenMemberType: Type;

/**
* The type of the own member with all type parameters of the own member substituted. Substitutions are based on the
* types of the corresponding parameters of the overridden member.
*/
substitutedOwnMemberType: Type;

/**
* The type of the overridden member with all type parameters of the overridden member substituted. Substitutions
* are based on the types of the corresponding parameters of the own member.
*/
substitutedOverriddenMemberType: Type;
}

const isInSafedsLangAnyClass = (services: SafeDsServices, node: SdsClassMember): boolean => {
const containingClass = getContainerOfType(node, isSdsClass);
return (
Expand Down
12 changes: 12 additions & 0 deletions packages/safe-ds-lang/tests/resources/typing/expressions/operations/arithmetic/main.sdstest
View file
Original file line number Diff line number Diff line change
Expand Up @@ -140,3 +140,15 @@ pipeline mixedOperands {
// $TEST$ serialization Float
val divisionFloatFloat = »1.5 / anyFloat()«;
}

// Strict checking of type parameter types
class MyClass<T sub Any>(
p1: T,

// $TEST$ serialization Float
p2: Any? = »p1 + 1«,
// $TEST$ serialization Float
p3: Any? = »1 + p1«,
// $TEST$ serialization Float
p4: Any? = »-p1«,
)
Loading

0 comments on commit 0e657cf

Please sign in to comment.