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

type foo = struct is unfamiliar #735

Closed
litherum opened this issue Apr 29, 2020 · 11 comments · Fixed by #952
Closed

type foo = struct is unfamiliar #735

litherum opened this issue Apr 29, 2020 · 11 comments · Fixed by #952
Assignees
Labels
wgsl resolved Resolved - waiting for a change to the WGSL specification wgsl WebGPU Shading Language Issues
Projects

Comments

@litherum
Copy link
Contributor

We should consider changing type foo = struct to struct foo instead. This is just as expressive, but more familiar to authors.

@litherum litherum added question wgsl WebGPU Shading Language Issues and removed question labels Apr 29, 2020
@litherum
Copy link
Contributor Author

I don't think there are any downsides to this; it seems free.

@kainino0x
Copy link
Contributor

FWIW it's rather similar to typedef struct { ... } MyStruct; from C.

@grorg grorg added this to Under Discussion in WGSL May 1, 2020
@grorg grorg moved this from Under Discussion to For Next Meeting in WGSL May 9, 2020
@dneto0
Copy link
Contributor

dneto0 commented May 26, 2020

Currently the only place a struct_decl may appear is as the right-hand-side of a type-alias.

A couple of questions:

  1. Where do you propose attributes go? Right now in the "type foo = struct ..." construct the RHS can have attributes.

  2. Is it important to have a concept of a struct without also binding a name to it? For example, should we be able to create structure value without using the name? Example: struct{a:i32,b:f32}(1,2.0) That seems handy from an orthogonality perspective, but admittedly unusual for C-family languages. Currently the grammar does not support this.

  3. I assume we'd still have the ability to alias, e.g.

    struct foo { a:i32, b:f32 };
    type bar = foo; // Still allowed by the grammar.
    // using: TYPE IDENT EQUAL type_decl
    // and type_decl as: IDENT

@litherum
Copy link
Contributor Author

Currently the only place a struct_decl may appear is as the right-hand-side of a type-alias.

A couple of questions:

  1. Where do you propose attributes go? Right now in the "type foo = struct ..." construct the RHS can have attributes.

I don't think it's particularly important. How about this:

struct Foo [[block]] { ... }
  1. Is it important to have a concept of a struct without also binding a name to it? For example, should we be able to create structure value without using the name? Example: struct{a:i32,b:f32}(1,2.0) That seems handy from an orthogonality perspective, but admittedly unusual for C-family languages. Currently the grammar does not support this.

The usage of this, even in C code, is so rare, that I don't think this complexity is worth it.

While we were working on WSL, one of the places where we benefitted from simplicity was the fact that all types were guaranteed by the grammar to be defined at the top level, so we didn't have to worry about scoping of types. All the types "lived forever" so we didn't have to do any magic when we inlined or outlined functions. I think, at least for now, the value of being able to do this in a compiler outweighs the value authors can (but rarely do!) get from anonymous structs.

  1. I assume we'd still have the ability to alias, e.g.
    struct foo { a:i32, b:f32 };
    type bar = foo; // Still allowed by the grammar.
    // using: TYPE IDENT EQUAL type_decl
    // and type_decl as: IDENT

Of course.

@kainino0x
Copy link
Contributor

How would anonymous structs go out of scope? Since they have no name, and names are what's scoped, anonymous structs have no scope.

In order to be able to use anonymous types at all, you have to have a concept of type equivalence ("this struct{a:i32} is the same as that struct{a:i32}") but I think this has to be true anyway ("this type a = x is the same as that type b = x")

@kainino0x
Copy link
Contributor

Another thought: anonymous structs are actually useful in C. They're most frequently used in unions, I think, but also appear in nested structs:

struct X {
  int length;
  struct {
    int x;
    int y;
  } points[MAX_LENGTH];
};

@johnkslang
Copy link

johnkslang commented May 27, 2020

How would anonymous structs go out of scope?

All the member names were entered into the scope instead, and that's what will then go out of scope.

Anonymous structs introduce complexities into tool chains, and possible ambiguities depending on name hiding rules:

int x...;
{ // new scope
  struct Y {
    int x; // x collides with the outer x
  };       // if it was not anonymous it would not collide
}

Will you do hiding here or give an error? Like, does the "non-name" of the struct avoid the name collision or not?

I think this is just sugar that adds complexity and recommend staying explicit.

@kainino0x
Copy link
Contributor

kainino0x commented May 27, 2020

I didn't realize that would cause a collision, and I don't totally understand it. But I don't need to. In any case, definitely fine with me to leave anonymous structs out.

@grorg
Copy link
Contributor

grorg commented Jun 2, 2020

Discussed at the 2020-06-02 meeting.

@grorg
Copy link
Contributor

grorg commented Jun 9, 2020

Discussed at the 2020-06-09 meeting.

@litherum
Copy link
Contributor Author

litherum commented Jun 9, 2020

Resolved: Accept the proposal

@litherum litherum moved this from For Next Meeting to Resolved: Needs Specification Work in WGSL Jun 9, 2020
@grorg grorg added wgsl resolved Resolved - waiting for a change to the WGSL specification and removed for wgsl meeting labels Jun 23, 2020
@dj2 dj2 self-assigned this Jul 24, 2020
WGSL automation moved this from Resolved: Needs Specification Work to Done Aug 4, 2020
kdashg pushed a commit that referenced this issue Aug 4, 2020
This CL removes the requirement for a struct to be the RHS of a `type`
statement and, instead, allows an IDENT to be provided in the struct
declaration.

The struct is still required to be defined in the module scope.

The [[block]] attribute remains on the LHS of the struct_decl.

```
[[block]] struct A { [[offset 0]] b : f32; }
```

Fixes #735
ben-clayton pushed a commit to ben-clayton/gpuweb that referenced this issue Sep 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wgsl resolved Resolved - waiting for a change to the WGSL specification wgsl WebGPU Shading Language Issues
Projects
WGSL
Done
Development

Successfully merging a pull request may close this issue.

6 participants