[shader-ast] Allow type inference of defn with arbitrary number of arguments#440
[shader-ast] Allow type inference of defn with arbitrary number of arguments#440sylee957 wants to merge 2 commits intothi-ng:developfrom
Conversation
![qlty-cloud-legacy[bot]](https://avatars.githubusercontent.com/in/9403?s=60&v=4){kind=small}
| 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>; |
There was a problem hiding this comment.
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>; |
There was a problem hiding this comment.
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>; |
There was a problem hiding this comment.
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>; |
There was a problem hiding this comment.
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>; |
There was a problem hiding this comment.
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>; |
There was a problem hiding this comment.
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>; |
There was a problem hiding this comment.
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>; |
There was a problem hiding this comment.
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< |
There was a problem hiding this comment.
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< |
There was a problem hiding this comment.
Similar blocks of code found in 2 locations. Consider refactoring.
|
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! |
2 participants
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 asSym<'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
anycasting that were used before.I wonder if there is more robust suggestion to make
Tround trip toSym<T>instead ofSym<TypeOfArg<T>>,which seems quite complicated to debug.
I think that the complexity comes from the fact that
argsis defined as unionT | [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-astdoes 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
outorinoutqualifier.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.