Skip to content

Conversation

@sylee957
Copy link

@sylee957 sylee957 commented Jan 1, 2024

It is possible to avoid overloads TaggedFn5, TaggedFn6, ... by using variadic tuple types and mapped types.
I try to demonstrate the implementation here.

Also, args: [...Args], is very useful here to infer the type as heterogeneous tuples than homogeneous arrays.
If I do args: Args, and if I pass array like ['int', 'float', 'bool'], the types of parameters are inferred as Sym<'int' | 'float' | 'bool'> which is not desirable.

As I check the integration with stdlib,
I had seen some failing cases of implementation that uses very generic types,
such as matching types of mul<TypeOfArg<T>, 'float'>.

However, I was able to fix the issues by extending some overloads,
and I also tested out by removing some other any casting that were used before.

I wonder if there is more robust suggestion to make T round trip to Sym<T> instead of Sym<TypeOfArg<T>>,
which seems quite complicated to debug.
I think that the complexity comes from the fact that args is defined as union T | [T, string, options] which is a bit involved to descompose.

The other rationale that I want to remove the restriction of number of parateters of function,
is that shader-ast does not support struct yet.
The only workaround to transform a program that uses struct, into a program that doesn't use struct, is
to expand struct into a lot of variables,
and to use functions with many parameters with out or inout qualifier.
And I worry about if I encouter such limitations in production, and if I want to avoid 'death by a thousand overloads' problem.
So it can be useful to remove such limitation to define functions more than 8 parameters.

export function add<T extends Int | "float">(l: Term<T>, r: number): Op2<T>;
// prettier-ignore
export function add<T extends Vec | Mat>(l: FloatTerm | number, r: Term<T>): Op2<T>;
export function add<T extends 'float' | Vec | Mat>(l: FloatTerm | number, r: Term<T>): Op2<T>;

Choose a reason for hiding this comment

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

Similar blocks of code found in 4 locations. Consider refactoring.

export function sub<T extends Int | "float">(l: Term<T>, r: number): Op2<T>;
// prettier-ignore
export function sub<T extends Vec | Mat>(l: FloatTerm | number, r: Term<T>): Op2<T>;
export function sub<T extends 'float' | Vec | Mat>(l: FloatTerm | number, r: Term<T>): Op2<T>;

Choose a reason for hiding this comment

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

Similar blocks of code found in 4 locations. Consider refactoring.

export function mul<T extends Int | "float">(l: Term<T>, r: number): Op2<T>;
// prettier-ignore
export function mul<T extends Vec | Mat>(l: FloatTerm | number, r: Term<T>): Op2<T>;
export function mul<T extends 'float' | Vec | Mat>(l: FloatTerm | number, r: Term<T>): Op2<T>;

Choose a reason for hiding this comment

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

Similar blocks of code found in 4 locations. Consider refactoring.

export function div<T extends Int | "float">(l: Term<T>, r: number): Op2<T>;
// prettier-ignore
export function div<T extends Vec | Mat>(l: FloatTerm | number, r: Term<T>): Op2<T>;
export function div<T extends 'float' | Vec | Mat>(l: FloatTerm | number, r: Term<T>): Op2<T>;

Choose a reason for hiding this comment

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

Similar blocks of code found in 4 locations. Consider refactoring.

export function add<T extends 'float' | Vec | Mat>(l: FloatTerm | number, r: Term<T>): Op2<T>;
// prettier-ignore
export function add<T extends Vec | Mat>(l: Term<T>, r: FloatTerm | number): Op2<T>;
export function add<T extends 'float' | Vec | Mat>(l: Term<T>, r: FloatTerm | number): Op2<T>;

Choose a reason for hiding this comment

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

Similar blocks of code found in 4 locations. Consider refactoring.

export function sub<T extends 'float' | Vec | Mat>(l: FloatTerm | number, r: Term<T>): Op2<T>;
// prettier-ignore
export function sub<T extends Vec | Mat>(l: Term<T>, r: FloatTerm | number): Op2<T>;
export function sub<T extends 'float' | Vec | Mat>(l: Term<T>, r: FloatTerm | number): Op2<T>;

Choose a reason for hiding this comment

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

Similar blocks of code found in 4 locations. Consider refactoring.

export function mul<T extends 'float' | Vec | Mat>(l: FloatTerm | number, r: Term<T>): Op2<T>;
// prettier-ignore
export function mul<T extends Vec | Mat>(l: Term<T>, r: FloatTerm | number): Op2<T>;
export function mul<T extends 'float' | Vec | Mat>(l: Term<T>, r: FloatTerm | number): Op2<T>;

Choose a reason for hiding this comment

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

Similar blocks of code found in 4 locations. Consider refactoring.

export function div<T extends 'float' | Vec | Mat>(l: FloatTerm | number, r: Term<T>): Op2<T>;
// prettier-ignore
export function div<T extends Vec | Mat>(l: Term<T>, r: FloatTerm | number): Op2<T>;
export function div<T extends 'float' | Vec | Mat>(l: Term<T>, r: FloatTerm | number): Op2<T>;

Choose a reason for hiding this comment

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

Similar blocks of code found in 4 locations. Consider refactoring.

/** @deprecated */
export type Arg2<A extends Type, B extends Type> = VariadicArgs<[A, B]>;
/** @deprecated */
export type Arg3<A extends Type, B extends Type, C extends Type> = VariadicArgs<

Choose a reason for hiding this comment

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

Similar blocks of code found in 2 locations. Consider refactoring.

/** @deprecated */
export type FnBody2<A extends Type, B extends Type> = VariadicFnBody<[A, B]>;
/** @deprecated */
export type FnBody3<

Choose a reason for hiding this comment

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

Similar blocks of code found in 2 locations. Consider refactoring.

@postspectacular
Copy link
Member

postspectacular commented Jan 3, 2024

My experience with using mapped types has been hit & miss over the years, esp. since they (TS team) keep on playing with inferencing rules and so making mapped types more brittle over time. A mapped type which worked in one TS version might not work in the future. Again (just as with template literal types in your other PR), a lot of this based on (repeated) experience with older (but not that old!) versions of TS and maybe most of these issues have been resolved or are more stable now, but I've literally wasted weeks of my life setting up complicated mapped type systems only to have them break completely in a TS upgrade a year later... But having said all that, mapped types are of course a beautiful thing (when they work) I will check our your branch and report back in a few days! So THANK YOU for that already!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants