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

Fix compiler crash related to using tuples as a generic constraint #4005

Merged
merged 1 commit into from
Feb 15, 2022

Conversation

SeanTAllen
Copy link
Member

@SeanTAllen SeanTAllen commented Feb 13, 2022

Tuple types aren't legal constraints on generics in Pony and we have a check in
the syntax pass to detect and report that error. However, it was previously
possible to sneak a tuple type as a generic constraint past the syntax check by
smuggling it in a type alias.

Type aliases aren't flattened into their various components until the flatten
pass which follows the syntax pass. This means that the following code would
cause a compiler assertion:

type Blocksize is (U8, U32)
class Block[T: Blocksize]

This commit adds an additional check in the flatten pass to report an error on
smuggled tuple types as generic constraints.

Fixes #3655

@SeanTAllen SeanTAllen requested a review from a team February 13, 2022 16:02
@SeanTAllen SeanTAllen added the changelog - fixed Automatically add "Fixed" CHANGELOG entry on merge label Feb 13, 2022
@ponylang-main ponylang-main added the discuss during sync Should be discussed during an upcoming sync label Feb 13, 2022
@SeanTAllen
Copy link
Member Author

Windows failure can be ignored.

@SeanTAllen SeanTAllen force-pushed the issue-3655 branch 4 times, most recently from f73e5d8 to 3254ad0 Compare February 15, 2022 19:20
@ergl
Copy link
Member

ergl commented Feb 15, 2022

I'd like to try if this catches something like the following:

type Blocksize is (None | (U8, U32))
class Block[T: Blocksize]

but I'm okay with merging now and fixing that edge case later

@SeanTAllen
Copy link
Member Author

@ergl i just tested, it doesn't fix that case. let me see how hard that is to address as well.

@SeanTAllen
Copy link
Member Author

@ergl can you open an issue for the type union one? its fairly easy to address but I'm too tired right now to deal with reworking the code to do the recursion necessary so I'm going to merge this now and then when the other issue is open, I or someone else can tackle.

Tuple types aren't legal constraints on generics in Pony and we have a check in
the syntax pass to detect and report that error. However, it was previously
possible to sneak a tuple type as a generic constraint past the syntax check by
smuggling it in a type alias.

Type alias aren't flattened into their various components until the flatten
pass which follows the syntax pass. This means that the following code would
cause a compiler assertion:

```ponyc
type Blocksize is (U8, U32)
class Block[T: Blocksize]
```

This commit adds an additional check in the flatten pass to report an error on
smuggled tuple types as generic constraints.

Fixes #3655
@SeanTAllen SeanTAllen merged commit 717a699 into main Feb 15, 2022
@SeanTAllen SeanTAllen deleted the issue-3655 branch February 15, 2022 20:45
@ponylang-main ponylang-main removed the discuss during sync Should be discussed during an upcoming sync label Feb 15, 2022
github-actions bot pushed a commit that referenced this pull request Feb 15, 2022
github-actions bot pushed a commit that referenced this pull request Feb 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog - fixed Automatically add "Fixed" CHANGELOG entry on merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Type alias AST backtrace
3 participants