Skip to content

Improve Signature lookups, Make Signature immutable #2582

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
Dec 5, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
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: 3 additions & 4 deletions src/compiler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6430,8 +6430,8 @@ export class Compiler extends DiagnosticEmitter {
assert(operandIndex == minOperands);

// create the varargs stub
stub = original.newStub("varargs");
stub.signature.requiredParameters = maxArguments;
stub = original.newStub("varargs", maxArguments);

original.varargsStub = stub;

// compile initializers of omitted arguments in the scope of the stub,
Expand Down Expand Up @@ -7102,8 +7102,7 @@ export class Compiler extends DiagnosticEmitter {
}
}

let signature = new Signature(this.program, parameterTypes, returnType, thisType);
signature.requiredParameters = numParameters; // !
let signature = new Signature(this.program, parameterTypes, returnType, thisType, numParameters);
instance = new Function(
prototype.name,
prototype,
Expand Down
6 changes: 3 additions & 3 deletions src/program.ts
Original file line number Diff line number Diff line change
Expand Up @@ -472,7 +472,7 @@ export class Program extends DiagnosticEmitter {
/** Managed classes contained in the program, by id. */
managedClasses: Map<i32,Class> = new Map();
/** A set of unique function signatures contained in the program, by id. */
uniqueSignatures: Signature[] = new Array<Signature>(0);
uniqueSignatures: Map<string, Signature> = new Map<string, Signature>();
/** Module exports. */
moduleExports: Map<string,Element> = new Map();
/** Module imports. */
Expand Down Expand Up @@ -3826,12 +3826,12 @@ export class Function extends TypedElement {
}

/** Creates a stub for use with this function, i.e. for varargs or override calls. */
newStub(postfix: string): Function {
newStub(postfix: string, requiredParameters: i32 = this.signature.requiredParameters): Function {
let stub = new Function(
this.original.name + STUB_DELIMITER + postfix,
this.prototype,
this.typeArguments,
this.signature.clone(),
this.signature.clone(requiredParameters),
this.contextualTypeArguments
);
stub.original = this.original;
Expand Down
16 changes: 10 additions & 6 deletions src/resolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -422,9 +422,7 @@ export class Resolver extends DiagnosticEmitter {
);
if (!returnType) return null;
}
let signature = new Signature(this.program, parameterTypes, returnType, thisType);
signature.requiredParameters = requiredParameters;
signature.hasRest = hasRest;
let signature = new Signature(this.program, parameterTypes, returnType, thisType, requiredParameters, hasRest);
return node.isNullable ? signature.type.asNullable() : signature.type;
}

Expand Down Expand Up @@ -2706,7 +2704,14 @@ export class Resolver extends DiagnosticEmitter {
}
const type = this.resolveExpression(expr, tempFlow, ctxType, reportMode);
if (type) {
signatureReference.returnType = type;
functionType.signatureReference = new Signature(
this.program,
signatureReference.parameterTypes,
type,
signatureReference.thisType,
signatureReference.requiredParameters,
signatureReference.hasRest,
);
}
}
return functionType;
Expand Down Expand Up @@ -2853,8 +2858,7 @@ export class Resolver extends DiagnosticEmitter {
returnType = type;
}

let signature = new Signature(this.program, parameterTypes, returnType, thisType);
signature.requiredParameters = requiredParameters;
let signature = new Signature(this.program, parameterTypes, returnType, thisType, requiredParameters);

let nameInclTypeParameters = prototype.name;
if (instanceKey.length) nameInclTypeParameters += `<${instanceKey}>`;
Expand Down
85 changes: 55 additions & 30 deletions src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -421,13 +421,20 @@ export class Type {

/** Tests if this type equals the specified. */
equals(other: Type): bool {
if (this.kind != other.kind) return false;
if (this.kind != other.kind) {
return false;
}
if (this.isReference) {
return (
this.classReference == other.classReference &&
this.signatureReference == other.signatureReference &&
this.isNullableReference == other.isNullableReference
);
if (this.classReference != other.classReference) return false;
let selfSignatureReference = this.signatureReference;
let otherSignatureReference = other.signatureReference;
if (selfSignatureReference && otherSignatureReference) {
if (!selfSignatureReference.equals(otherSignatureReference)) return false;
} else {
if (selfSignatureReference != otherSignatureReference) return false;
}

return this.isNullableReference == other.isNullableReference;
}
return true;
}
Expand Down Expand Up @@ -886,14 +893,10 @@ export class Signature {
id: u32 = 0;
/** Parameter types, if any, excluding `this`. */
parameterTypes: Type[];
/** Number of required parameters excluding `this`. Other parameters are considered optional. */
requiredParameters: i32;
/** Return type. */
returnType: Type;
/** This type, if an instance signature. */
thisType: Type | null;
/** Whether the last parameter is a rest parameter. */
hasRest: bool;
/** Respective function type. */
type: Type;
/** The program that created this signature. */
Expand All @@ -904,14 +907,16 @@ export class Signature {
program: Program,
parameterTypes: Type[] | null = null,
returnType: Type | null = null,
thisType: Type | null = null
thisType: Type | null = null,
/** Number of required parameters excluding `this`. Other parameters are considered optional. */
public readonly requiredParameters: i32 = parameterTypes ? parameterTypes.length : 0,
/** Whether the last parameter is a rest parameter. */
public readonly hasRest: bool = false,
) {
this.parameterTypes = parameterTypes ? parameterTypes : [];
this.requiredParameters = 0;
this.returnType = returnType ? returnType : Type.void;
this.thisType = thisType;
this.program = program;
this.hasRest = false;
let usizeType = program.options.usizeType;
let type = new Type(
usizeType.kind,
Expand All @@ -922,16 +927,20 @@ export class Signature {
type.signatureReference = this;

let signatureTypes = program.uniqueSignatures;
let length = signatureTypes.length;
for (let i = 0; i < length; i++) {
let compare = unchecked(signatureTypes[i]);
if (this.equals(compare)) {
this.id = compare.id;
return this;
let compareString = this.toString();
if (signatureTypes.has(compareString)) {
let existing = signatureTypes.get(compareString)!;
if (!this.equals(existing)) {
trace(compareString);
trace(existing.toString());
}
assert(this.equals(existing), "Types don't match");
this.id = existing.id;
return this;
}

this.id = program.nextSignatureId++;
signatureTypes.push(this);
signatureTypes.set(compareString, this);
}

get paramRefs(): TypeRef {
Expand Down Expand Up @@ -965,25 +974,38 @@ export class Signature {
if (thisThisType) {
if (!otherThisType || !thisThisType.equals(otherThisType)) return false;
} else if (otherThisType) {
trace("failed on thisThisType");
return false;
}

// check rest parameter
if (this.hasRest != other.hasRest) return false;
if (this.hasRest != other.hasRest) {
trace("failed on hasRest");
return false;
}

// check return type
if (!this.returnType.equals(other.returnType)) return false;
if (!this.returnType.equals(other.returnType)) {
trace("failed on returnType");
return false;
}

// check parameter types
let thisParameterTypes = this.parameterTypes;
let selfParameterTypes = this.parameterTypes;
let otherParameterTypes = other.parameterTypes;
let numParameters = thisParameterTypes.length;
if (numParameters != otherParameterTypes.length) return false;
let numParameters = selfParameterTypes.length;
if (numParameters != otherParameterTypes.length) {
trace("failed on numParameters");
return false;
}

for (let i = 0; i < numParameters; ++i) {
let thisParameterType = unchecked(thisParameterTypes[i]);
let selfParameterType = unchecked(selfParameterTypes[i]);
let otherParameterType = unchecked(otherParameterTypes[i]);
if (!thisParameterType.equals(otherParameterType)) return false;
if (!selfParameterType.equals(otherParameterType)) {
trace("failed on selfParameterType");
return false;
}
}
return true;
}
Expand Down Expand Up @@ -1105,7 +1127,8 @@ export class Signature {
let thisType = this.thisType;
if (thisType) {
sb.push(validWat ? "this:" : "this: ");
assert(!thisType.signatureReference);
// sometimes "this" can be a function type
// assert(!thisType.signatureReference);
sb.push(thisType.toString(validWat));
index = 1;
}
Expand All @@ -1127,7 +1150,7 @@ export class Signature {
}

/** Creates a clone of this signature that is safe to modify. */
clone(): Signature {
clone(requieredParameters: i32 = this.requiredParameters, hasRest: bool = this.hasRest): Signature {
let parameterTypes = this.parameterTypes;
let numParameterTypes = parameterTypes.length;
let cloneParameterTypes = new Array<Type>(numParameterTypes);
Expand All @@ -1138,7 +1161,9 @@ export class Signature {
this.program,
cloneParameterTypes,
this.returnType,
this.thisType
this.thisType,
requieredParameters,
hasRest
);
}
}
8 changes: 4 additions & 4 deletions tests/compiler/builtins.debug.wat
Original file line number Diff line number Diff line change
Expand Up @@ -3213,11 +3213,11 @@
local.set $48
i32.const 0
local.set $49
i32.const 46
i32.const 51
local.set $50
i32.const 47
i32.const 52
local.set $51
i32.const 47
i32.const 52
local.set $52
i32.const 256
local.set $63
Expand Down Expand Up @@ -3262,7 +3262,7 @@
unreachable
end
local.get $50
i32.const 46
i32.const 51
i32.eq
i32.eqz
if
Expand Down
6 changes: 3 additions & 3 deletions tests/compiler/builtins.release.wat
Original file line number Diff line number Diff line change
Expand Up @@ -735,9 +735,9 @@
i32.const 5
f64.const 0
f64.const 0
f64.const 46
f64.const 47
f64.const 47
f64.const 51
f64.const 52
f64.const 52
call $~lib/builtins/trace
global.get $~lib/memory/__stack_pointer
local.tee $0
Expand Down