Skip to content
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

Generalize the handling of generics in JSON ABI #2386

Closed
mohammadfawaz opened this issue Jul 26, 2022 · 3 comments · Fixed by #2524
Closed

Generalize the handling of generics in JSON ABI #2386

mohammadfawaz opened this issue Jul 26, 2022 · 3 comments · Fixed by #2524
Assignees
Labels
ABI Everything to do the ABI, especially the JSON representation P: critical Should be looked at before anything else

Comments

@mohammadfawaz
Copy link
Contributor

mohammadfawaz commented Jul 26, 2022

#2218 introduces the typeArgument field to the JSON ABI which got us one step closer to supporting (resolved) generics in the ABI. However, as @iqdecay pointed out, this doesn't solve the full problem because even though we know what type parameters a given struct/enum has and what they resolve to, we do not know what generic parameter each component actually uses. For example:

struct MyStruct1<T, U> {
    x: T,
    y: U
}

struct MyStruct2<T, U> {
    x: U,
    y: T
}

fn some_abi_funct(s1: MyStruct1<u64, u64>, s2: MyStruct2<u64, u64>);

Both structs above would be represented in the same way in the JSON ABI (except for the name of course) and that's ambiguous.

The typeArguments field gave us half the solution. The other half is that we should modify the type field to contain the type parameter applied. For example, the first input would be represented as follows:

      {
        "name": "s1",
        "type": "struct MyStruct1<T, U>",
        "components": [
          {
            "name": "x",
            "type": "T",
            "components": null,
            "typeArguments": null
          },
          {
            "name": "y",
            "type": "U",
            "components": null,
            "typeArguments": null
          }
        ],
        "typeArguments": [
          {
            "name": "T",
            "type": "u64",
            "components": null,
            "typeArguments": null
          },
          {
            "name": "U",
            "type": "u64",
            "components": null,
            "typeArguments": null
          }
        ]
      },

The above has all the information we need to recreate the generic struct declaration and also recreate its specialization (i.e. MyStruct1<u64, u64>).

This gets a bit more complicated when the field of a struct is a complex type that uses one of the generics. For example:

struct MyStruct2<T, U> {
    x: U,
    y: T
}

struct MyStruct3<V, W> {
    x:  MyStruct2<V, W>,
}

fn some_other_abi_funct(s3: MyStruct3<bool, u64>);

but the same concepts still applies:

      {
        "name": "s1",
        "type": "struct MyStruct3<V, W>",
        "components": [
          {
            "name": "x",
            "type": "struct MyStruct2<V, W>",
            "components": [
              {
                "name": "x",
                "type": "U",
                "components": null,
                "typeArguments": null
              },
              {
                "name": "y",
                "type": "T",
                "components": null,
                "typeArguments": null
              }
            ],
            "typeArguments": [
              {
                "name": "T",
                "type": "u64",
                "components": null,
                "typeArguments": null
              },
              {
                "name": "U",
                "type": "u64",
                "components": null,
                "typeArguments": null
              }
            ]
          }
        ],
        "typeArguments": [
          {
            "name": "V",
            "type": "u64",
            "components": null,
            "typeArguments": null
          },
          {
            "name": "W",
            "type": "u64",
            "components": null,
            "typeArguments": null
          }
        ]
      }

This introduces no changes to non-generic types so the current working flows continue to work as expected.

@mohammadfawaz mohammadfawaz added bug Something isn't working ABI Everything to do the ABI, especially the JSON representation P: critical Should be looked at before anything else labels Jul 26, 2022
@mohammadfawaz mohammadfawaz self-assigned this Jul 27, 2022
@mohammadfawaz
Copy link
Contributor Author

mohammadfawaz commented Aug 3, 2022

Another option is to completely redesign the JSON ABI as suggested by @emilyaherbert:

Quoting Emily:

This is what I would ideally like to see for the ABI, but I don't immediately think it is tractable to do this with the current state of collection context stuff probably tractable!.
Given this struct:

MyStruct<T,U> {
    bim: T,
    bam: U,
    never_used: u64,
}

this ABI method:

fn add(foo: MyStruct<u64, bool>, bar: MyStruct<u64, u8>);

it would be cool if we could achieve something similar to this:

{
  "types": [
    {                                       // 0
      "type": "u64",
      "typeParameters": null,
      "components": null
    },
    {                                       // 1
      "type": "bool",
      "typeParameters": null,
      "components": null
    },
    {                                       // 2
      "type": "u8",
      "typeParameters": null,
      "components": null
    },
    {                                       // 3
      "type": "generic T0",
      "typeParameters": null,
      "components": null
    },
    {                                       // 4
      "type": "generic U0",
      "typeParameters": null,
      "components": null
    },
    {                                       // 5
      "type": "struct MyStruct",
      "typeParameters": [3, 4],
      "components": [
        {
          "name": "bim",
          "type": 3,
          "typeArguments": null
        },
        {
          "name": "bam",
          "type": 4,
          "typeArguments": null
        },
        {
          "name": "never_used",
          "type": 0,
          "typeArguments": null
        }
      ]
    },
    {                                       // 6
      "type": "()",
      "typeParameters": null,
      "components": null
    }
  ],
  "functions": [
    {
      "type": "function",
      "inputs": [
        {
          "name": "foo",
          "type": 5,
          "typeArguments": [0, 1]
        },
        {
          "name": "bar",
          "type": 5,
          "typeArguments": [0, 2]
        }
      ],
      "output": [
        {
          "name": "",
          "type": 6,
          "typeArguments": null 
        }
      ]
    }
  ]
}

@mohammadfawaz
Copy link
Contributor Author

A third short term option is to work with what we have today (i.e. typeArguments alone) and special case things for Option, Result, and Vec. This requires no changes in the compiler and probably minimal non-breaking changes in both SDKs. It also covers 99% of the cases that users care about in the short term. We can then take our time implementing option 2 above which will help with #2130.

@mohammadfawaz
Copy link
Contributor Author

We agreed to go with the flat JSON ABI design for it's simplicity and clarity. The types now go first then the functions.

There are basically two types of properties (unlike before):

  1. Type 1 is a type declaration that has “type”, “typeParameters”, “components”. Those have IDs and show up in the “types” array.
  2. Type 2 is a type application that has “name”, “type”, “typeArguments”. Those use the type declarations above (in the ABI directly or in components such as struct fields) and apply type arguments if needed.
    All the types (starting with just ABI types for now) are listed first. Then all the “functions” follow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ABI Everything to do the ABI, especially the JSON representation P: critical Should be looked at before anything else
Projects
None yet
1 participant