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.

Sign up for GitHub

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

Jump to bottom

[shader-ast] Allow type inference of defn with arbitrary number of arguments #440

Open

sylee957 wants to merge 2 commits into thi-ng:develop from sylee957:shader-ast/variadic

sylee957 commented Jan 1, 2024 •

edited

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.


          Allow type inference of defn with arbitrary number of arguments

6a5d9a0

codeclimate bot reviewed

View reviewed changes

packages/shader-ast/src/ast/ops.ts

               export function add<T extends Int | "float">(l: number, r: Term<T>): Op2<T>;
               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>;

codeclimate bot Jan 1, 2024

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

packages/shader-ast/src/ast/ops.ts

               export function sub<T extends Int | "float">(l: number, r: Term<T>): Op2<T>;
               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>;

codeclimate bot Jan 1, 2024

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

packages/shader-ast/src/ast/ops.ts

               export function mul<T extends Int | "float">(l: number, r: Term<T>): Op2<T>;
               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>;

codeclimate bot Jan 1, 2024

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

packages/shader-ast/src/ast/ops.ts

               export function div<T extends Int | "float">(l: number, r: Term<T>): Op2<T>;
               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>;

codeclimate bot Jan 1, 2024

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

packages/shader-ast/src/ast/ops.ts

               // 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>;

codeclimate bot Jan 1, 2024

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


          Remove redudant type parameter

fe3ddb6

codeclimate bot reviewed

View reviewed changes

packages/shader-ast/src/ast/ops.ts

               // 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>;

codeclimate bot Jan 1, 2024

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

packages/shader-ast/src/ast/ops.ts

               // 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>;

codeclimate bot Jan 1, 2024

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

packages/shader-ast/src/ast/ops.ts

               // 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>;

codeclimate bot Jan 1, 2024

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

packages/shader-ast/src/api/function.ts

+              /** @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<

codeclimate bot Jan 1, 2024

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

packages/shader-ast/src/api/function.ts

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

codeclimate bot Jan 1, 2024

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

Member

postspectacular commented Jan 3, 2024 •

edited

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