tuple indexes

tuple indexes

[Mark Wielaard]

[-- Attachment #1: Type: text/plain, Size: 1375 bytes --]

Hi,

I was looking into https://github.com/Rust-GCC/gccrs/issues/511 "rust
has two kinds of integer literal" Which explains that integer literals
used for a tuple index are not general integer literals.

First I wanted to write some tests, and started with some constructs
that should pass. But some don't. In particular the empty tuple struct
isn't recognized, and the struct name path lookup doesn't work when
initializing the tuple struct.

tuple_index.rs:16:12: error: unrecognised token ‘)’ in type
   16 |   struct E();
      |            ^
tuple_index.rs:16:12: error: could not parse type in tuple struct field

tuple_index.rs:20:12: error: unknown root segment in path O lookup O
   20 |   let so = O(0);
      |            ^
tuple_index.rs:24:12: error: unknown root segment in path T lookup T
   24 |   let st = T(0,1);
      |            ^
tuple_index.rs:28:12: error: unknown root segment in path M lookup M
   28 |   let sm = M(0,1,2,3,4,5,6,7,8,9,10);
      |            ^

I haven't had time to try to resolve these issues, but wanted to
report them.

Finally, the The Rust Reference says "A tuple index is used to refer
to the fields of tuples, tuple structs, and tuple variants." I don't
understand how this would work for tuple variants. Does anybody have
an example of how to refer to a tuple variant so a tuple index can be
used on it?

Cheers,

Mark

[-- Attachment #2: tuple_index.rs --]
[-- Type: application/rls-services+xml, Size: 468 bytes --]

Re: tuple indexes

[Philip Herron]

[-- Attachment #1.1: Type: text/plain, Size: 2111 bytes --]

On 22/06/2021 23:51, Mark Wielaard wrote:
> Hi,
>
> I was looking into https://github.com/Rust-GCC/gccrs/issues/511 "rust
> has two kinds of integer literal" Which explains that integer literals
> used for a tuple index are not general integer literals.
>
> First I wanted to write some tests, and started with some constructs
> that should pass. But some don't. In particular the empty tuple struct
> isn't recognized, and the struct name path lookup doesn't work when
> initializing the tuple struct.
>
> tuple_index.rs:16:12: error: unrecognised token ‘)’ in type
>    16 |   struct E();
>       |            ^
> tuple_index.rs:16:12: error: could not parse type in tuple struct field
>
> tuple_index.rs:20:12: error: unknown root segment in path O lookup O
>    20 |   let so = O(0);
>       |            ^
> tuple_index.rs:24:12: error: unknown root segment in path T lookup T
>    24 |   let st = T(0,1);
>       |            ^
> tuple_index.rs:28:12: error: unknown root segment in path M lookup M
>    28 |   let sm = M(0,1,2,3,4,5,6,7,8,9,10);
>       |            ^
>
> I haven't had time to try to resolve these issues, but wanted to
> report them.
>
> Finally, the The Rust Reference says "A tuple index is used to refer
> to the fields of tuples, tuple structs, and tuple variants." I don't
> understand how this would work for tuple variants. Does anybody have
> an example of how to refer to a tuple variant so a tuple index can be
> used on it?
>
> Cheers,
>
> Mark
>
Hi Mark,

Good find, I have raised https://github.com/Rust-GCC/gccrs/issues/519.
From the top of my head I think there are a few issues going on.

1. The parser is not able to parse the structure definitions within a
block properly.

2. The knock on is that the name resolution and type resolution will
need updated to handle this.

I think I might take a quick look into this one today I want to double
check a few things as it may have a knock on as to what I am working on.
If i make any progress I will update the ticket and let you know here.

Thanks,

--Phil



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 665 bytes --]

Re: tuple indexes

[Philip Herron]

[-- Attachment #1.1: Type: text/plain, Size: 2582 bytes --]

On 23/06/2021 10:47, Philip Herron wrote:
> On 22/06/2021 23:51, Mark Wielaard wrote:
>> Hi,
>>
>> I was looking into https://github.com/Rust-GCC/gccrs/issues/511 "rust
>> has two kinds of integer literal" Which explains that integer literals
>> used for a tuple index are not general integer literals.
>>
>> First I wanted to write some tests, and started with some constructs
>> that should pass. But some don't. In particular the empty tuple struct
>> isn't recognized, and the struct name path lookup doesn't work when
>> initializing the tuple struct.
>>
>> tuple_index.rs:16:12: error: unrecognised token ‘)’ in type
>>    16 |   struct E();
>>       |            ^
>> tuple_index.rs:16:12: error: could not parse type in tuple struct field
>>
>> tuple_index.rs:20:12: error: unknown root segment in path O lookup O
>>    20 |   let so = O(0);
>>       |            ^
>> tuple_index.rs:24:12: error: unknown root segment in path T lookup T
>>    24 |   let st = T(0,1);
>>       |            ^
>> tuple_index.rs:28:12: error: unknown root segment in path M lookup M
>>    28 |   let sm = M(0,1,2,3,4,5,6,7,8,9,10);
>>       |            ^
>>
>> I haven't had time to try to resolve these issues, but wanted to
>> report them.
>>
>> Finally, the The Rust Reference says "A tuple index is used to refer
>> to the fields of tuples, tuple structs, and tuple variants." I don't
>> understand how this would work for tuple variants. Does anybody have
>> an example of how to refer to a tuple variant so a tuple index can be
>> used on it?
>>
>> Cheers,
>>
>> Mark
>>
> Hi Mark,
>
> Good find, I have raised https://github.com/Rust-GCC/gccrs/issues/519.
> From the top of my head I think there are a few issues going on.
>
> 1. The parser is not able to parse the structure definitions within a
> block properly.
>
> 2. The knock on is that the name resolution and type resolution will
> need updated to handle this.
>
> I think I might take a quick look into this one today I want to double
> check a few things as it may have a knock on as to what I am working on.
> If i make any progress I will update the ticket and let you know here.
>
> Thanks,
>
> --Phil
>
Small update, I think part of this issue is that the support for
unit-structs is not there yet in the compiler, so if you remove the unit
tuple struct and move the other struct definitions outside of the block
the test case works: https://godbolt.org/z/nb84sEaE4

It might be enough to help with testing your tuple index fixes.

Thanks

--Phil



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 665 bytes --]

Re: tuple indexes

[Mark Wielaard]

[-- Attachment #1: Type: text/plain, Size: 658 bytes --]

Hi Philip,

On Wed, 2021-06-23 at 10:55 +0100, Philip Herron wrote:
> Small update, I think part of this issue is that the support for
> unit-structs is not there yet in the compiler, so if you remove the unit
> tuple struct and move the other struct definitions outside of the block
> the test case works: https://godbolt.org/z/nb84sEaE4
> 
> It might be enough to help with testing your tuple index fixes.

Very nice. That workaround helps me get unblocked, except for testing
empty struct tuples (unit-structs). But I have a patch for that, and
even included a testcase to proof the parser now handles them. See
attached.

Cheers,

Mark

[-- Attachment #2: 0001-Handle-empty-unit-tuple-structs-in-the-parser.patch --]
[-- Type: text/x-patch, Size: 1939 bytes --]

From a1d03b5182f7e2b16b5201459c98b33a12775888 Mon Sep 17 00:00:00 2001
From: Mark Wielaard <mark@klomp.org>
Date: Wed, 23 Jun 2021 17:59:30 +0200
Subject: [PATCH] Handle empty/unit tuple structs in the parser.

A tuple struct can be empty, in which case it is a unit tuple struct.
Handle this in Parser<ManagedTokenSource>::parse_struct by creating
a empty tuple_field vector instead of calling parse_tuple_fields.

Add a testcase to show empty tuple structs are now accepted.
---
 gcc/rust/parse/rust-parse-impl.h                      |  7 ++++++-
 .../rust/compile/torture/tuple_struct_unit.rs         | 11 +++++++++++
 2 files changed, 17 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/rust/compile/torture/tuple_struct_unit.rs

diff --git a/gcc/rust/parse/rust-parse-impl.h b/gcc/rust/parse/rust-parse-impl.h
index b4f264e0261..dfac00e3dbc 100644
--- a/gcc/rust/parse/rust-parse-impl.h
+++ b/gcc/rust/parse/rust-parse-impl.h
@@ -3980,7 +3980,12 @@ Parser<ManagedTokenSource>::parse_struct (AST::Visibility vis,
       lexer.skip_token ();
 
       // parse tuple fields
-      std::vector<AST::TupleField> tuple_fields = parse_tuple_fields ();
+      std::vector<AST::TupleField> tuple_fields;
+      // Might be empty tuple for unit tuple struct.
+      if (lexer.peek_token ()->get_id () == RIGHT_PAREN)
+	tuple_fields = std::vector<AST::TupleField> ();
+      else
+	tuple_fields = parse_tuple_fields ();
 
       // tuple parameters must have closing parenthesis
       if (!skip_token (RIGHT_PAREN))
diff --git a/gcc/testsuite/rust/compile/torture/tuple_struct_unit.rs b/gcc/testsuite/rust/compile/torture/tuple_struct_unit.rs
new file mode 100644
index 00000000000..cda19d2af0b
--- /dev/null
+++ b/gcc/testsuite/rust/compile/torture/tuple_struct_unit.rs
@@ -0,0 +1,11 @@
+struct E();
+struct T(E,E,());
+
+fn main()
+{
+  let z0 = E();
+  let z1 = E();
+  let t = T(z0,z1,());
+  let z = t.2;
+  z
+}
-- 
2.31.1

Re: tuple indexes

[Philip Herron]

[-- Attachment #1.1: Type: text/plain, Size: 848 bytes --]

On 23/06/2021 17:06, Mark Wielaard wrote:
> Hi Philip,
>
> On Wed, 2021-06-23 at 10:55 +0100, Philip Herron wrote:
>> Small update, I think part of this issue is that the support for
>> unit-structs is not there yet in the compiler, so if you remove the unit
>> tuple struct and move the other struct definitions outside of the block
>> the test case works: https://godbolt.org/z/nb84sEaE4
>>
>> It might be enough to help with testing your tuple index fixes.
> Very nice. That workaround helps me get unblocked, except for testing
> empty struct tuples (unit-structs). But I have a patch for that, and
> even included a testcase to proof the parser now handles them. See
> attached.
>
> Cheers,
>
> Mark

This was a good find. Your patch is now being merged:
https://github.com/Rust-GCC/gccrs/pull/521

Thanks

--Phil



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 665 bytes --]

Re: tuple indexes

[Mark Wielaard]

[-- Attachment #1: Type: text/plain, Size: 2821 bytes --]

On Wed, Jun 23, 2021 at 12:51:34AM +0200, Mark Wielaard wrote:
> Finally, the The Rust Reference says "A tuple index is used to refer
> to the fields of tuples, tuple structs, and tuple variants." I don't
> understand how this would work for tuple variants. Does anybody have
> an example of how to refer to a tuple variant so a tuple index can be
> used on it?

Tom pointed out on irc that it doesn't seem possible to access enum
variant types except through a matching expression. The Rust Reference
also seems to be inconsistent. As mentioned above it mentions you can
use a tuple index to refer to a field of a enum tuple variant. But it
also says "A tuple indexing expression accesses fields of tuples and
tuple structs." So it probably really isn't possible to use a tuple
index on enum tuple variants.

I did notice the same issue as for unit tuple struct types. The empty
tuple wasn't accepted in the parser. The attached patch, also at
https://code.wildebeest.org/git/user/mjw/gccrs/commit/?h=tuple_enum_variant_unit
fixes this.

It does include a test case, but most of it is commented out because
actually resolving enum types isn't implemented yet. If you uncomment
the rest of the testcase you get:

tuple_enum_variants.rs:3:31: error: failed to resolve TypePath: E
    3 | fn f(e0: E, e1: E, e2: E) -> (E,E,E,())
      |                               ^
tuple_enum_variants.rs:3:31: error: unresolved type
tuple_enum_variants.rs:3:33: error: failed to resolve TypePath: E
    3 | fn f(e0: E, e1: E, e2: E) -> (E,E,E,())
      |                                 ^
tuple_enum_variants.rs:3:33: error: unresolved type
tuple_enum_variants.rs:3:35: error: failed to resolve TypePath: E
    3 | fn f(e0: E, e1: E, e2: E) -> (E,E,E,())
      |                                   ^
tuple_enum_variants.rs:3:35: error: unresolved type
tuple_enum_variants.rs:3:10: error: failed to resolve TypePath: E
    3 | fn f(e0: E, e1: E, e2: E) -> (E,E,E,())
      |          ^
tuple_enum_variants.rs:3:10: error: unresolved type
tuple_enum_variants.rs:3:17: error: failed to resolve TypePath: E
    3 | fn f(e0: E, e1: E, e2: E) -> (E,E,E,())
      |                 ^
tuple_enum_variants.rs:3:17: error: unresolved type
tuple_enum_variants.rs:3:24: error: failed to resolve TypePath: E
    3 | fn f(e0: E, e1: E, e2: E) -> (E,E,E,())
      |                        ^
tuple_enum_variants.rs:3:24: error: unresolved type
tuple_enum_variants.rs:13:12: error: unknown root segment in path E::T0 lookup E
   13 |   let e0 = E::T0();
      |            ^
tuple_enum_variants.rs:14:12: error: unknown root segment in path E::T1 lookup E
   14 |   let e1 = E::T1(0);
      |            ^
tuple_enum_variants.rs:15:12: error: unknown root segment in path E::T2 lookup E
   15 |   let e2 = E::T2(0,1);
      |            ^

Cheers,

Mark

[-- Attachment #2: 0001-Handle-empty-unit-tuple-enum-variants-in-the-parser.patch --]
[-- Type: text/x-diff, Size: 2141 bytes --]

From ee1d3b6ab4d508caea35efef67ec89f54178781c Mon Sep 17 00:00:00 2001
From: Mark Wielaard <mark@klomp.org>
Date: Wed, 23 Jun 2021 21:54:16 +0200
Subject: [PATCH] Handle empty/unit tuple enum variants in the parser.

A tuple enum variant can be empty, in which case it is a unit enum variant.
Handle this in Parser<ManagedTokenSource>::parse_enum_item by creating
a empty tuple_field vector instead of calling parse_tuple_fields.

Add a testcase to show empty tuple enum variant types are now accepted.
But note some part of the test is commented out because using the enum
type isn't actually possible right now.
---
 gcc/rust/parse/rust-parse-impl.h              |  7 ++++++-
 .../compile/torture/tuple_enum_variants.rs    | 19 +++++++++++++++++++
 2 files changed, 25 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/rust/compile/torture/tuple_enum_variants.rs

diff --git a/gcc/rust/parse/rust-parse-impl.h b/gcc/rust/parse/rust-parse-impl.h
index dfac00e3dbc..9a28f6cdb66 100644
--- a/gcc/rust/parse/rust-parse-impl.h
+++ b/gcc/rust/parse/rust-parse-impl.h
@@ -4415,7 +4415,12 @@ Parser<ManagedTokenSource>::parse_enum_item ()
 	// tuple enum item
 	lexer.skip_token ();
 
-	std::vector<AST::TupleField> tuple_fields = parse_tuple_fields ();
+	std::vector<AST::TupleField> tuple_fields;
+	// Might be empty tuple for unit tuple enum variant.
+	if (lexer.peek_token ()->get_id () == RIGHT_PAREN)
+	  tuple_fields = std::vector<AST::TupleField> ();
+	else
+	  tuple_fields = parse_tuple_fields ();
 
 	if (!skip_token (RIGHT_PAREN))
 	  {
diff --git a/gcc/testsuite/rust/compile/torture/tuple_enum_variants.rs b/gcc/testsuite/rust/compile/torture/tuple_enum_variants.rs
new file mode 100644
index 00000000000..26e3e5d0a71
--- /dev/null
+++ b/gcc/testsuite/rust/compile/torture/tuple_enum_variants.rs
@@ -0,0 +1,19 @@
+enum E { T0(), T1(i32), T2(i32,u32) }
+
+/* The following doesn't parse yet...
+fn f(e0: E, e1: E, e2: E) -> (E,E,E,())
+{
+  let e = e0;
+  let f = e1;
+  let g = e2;
+  (e,f,g,())
+}
+
+fn main()
+{
+  let e0 = E::T0();
+  let e1 = E::T1(0);
+  let e2 = E::T2(0,1);
+  f(e0, e1, e2).3
+}
+*/
-- 
2.32.0

Re: tuple indexes

[Philip Herron]

[-- Attachment #1.1: Type: text/plain, Size: 3366 bytes --]

On 23/06/2021 21:15, Mark Wielaard wrote:
> On Wed, Jun 23, 2021 at 12:51:34AM +0200, Mark Wielaard wrote:
>> Finally, the The Rust Reference says "A tuple index is used to refer
>> to the fields of tuples, tuple structs, and tuple variants." I don't
>> understand how this would work for tuple variants. Does anybody have
>> an example of how to refer to a tuple variant so a tuple index can be
>> used on it?
> Tom pointed out on irc that it doesn't seem possible to access enum
> variant types except through a matching expression. The Rust Reference
> also seems to be inconsistent. As mentioned above it mentions you can
> use a tuple index to refer to a field of a enum tuple variant. But it
> also says "A tuple indexing expression accesses fields of tuples and
> tuple structs." So it probably really isn't possible to use a tuple
> index on enum tuple variants.
>
> I did notice the same issue as for unit tuple struct types. The empty
> tuple wasn't accepted in the parser. The attached patch, also at
> https://code.wildebeest.org/git/user/mjw/gccrs/commit/?h=tuple_enum_variant_unit
> fixes this.
>
> It does include a test case, but most of it is commented out because
> actually resolving enum types isn't implemented yet. If you uncomment
> the rest of the testcase you get:
>
> tuple_enum_variants.rs:3:31: error: failed to resolve TypePath: E
>     3 | fn f(e0: E, e1: E, e2: E) -> (E,E,E,())
>       |                               ^
> tuple_enum_variants.rs:3:31: error: unresolved type
> tuple_enum_variants.rs:3:33: error: failed to resolve TypePath: E
>     3 | fn f(e0: E, e1: E, e2: E) -> (E,E,E,())
>       |                                 ^
> tuple_enum_variants.rs:3:33: error: unresolved type
> tuple_enum_variants.rs:3:35: error: failed to resolve TypePath: E
>     3 | fn f(e0: E, e1: E, e2: E) -> (E,E,E,())
>       |                                   ^
> tuple_enum_variants.rs:3:35: error: unresolved type
> tuple_enum_variants.rs:3:10: error: failed to resolve TypePath: E
>     3 | fn f(e0: E, e1: E, e2: E) -> (E,E,E,())
>       |          ^
> tuple_enum_variants.rs:3:10: error: unresolved type
> tuple_enum_variants.rs:3:17: error: failed to resolve TypePath: E
>     3 | fn f(e0: E, e1: E, e2: E) -> (E,E,E,())
>       |                 ^
> tuple_enum_variants.rs:3:17: error: unresolved type
> tuple_enum_variants.rs:3:24: error: failed to resolve TypePath: E
>     3 | fn f(e0: E, e1: E, e2: E) -> (E,E,E,())
>       |                        ^
> tuple_enum_variants.rs:3:24: error: unresolved type
> tuple_enum_variants.rs:13:12: error: unknown root segment in path E::T0 lookup E
>    13 |   let e0 = E::T0();
>       |            ^
> tuple_enum_variants.rs:14:12: error: unknown root segment in path E::T1 lookup E
>    14 |   let e1 = E::T1(0);
>       |            ^
> tuple_enum_variants.rs:15:12: error: unknown root segment in path E::T2 lookup E
>    15 |   let e2 = E::T2(0,1);
>       |            ^
>
> Cheers,
>
> Mark
>
Hi Mark,

Thanks for the patch, its being merged:
https://github.com/Rust-GCC/gccrs/pull/522

I have open issues about enums and unions they will be fixed as part of
my work into traits. They are a type of algebraic data type so in theory
i can reuse a lot of the existing code to implement them.

Thanks

--Phil



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 665 bytes --]