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

Static arrays. #424

Merged
merged 7 commits into from
Nov 29, 2021
Merged

Static arrays. #424

merged 7 commits into from
Nov 29, 2021

Conversation

otrho
Copy link
Contributor

@otrho otrho commented Nov 26, 2021

Here's an initial implementation of static arrays. This has taken me a while and it's arguably not yet finished but I want to get this going sooner rather than later.

The basics work -- declaring both [1, 2, 3] or [false; 3] style initialisers, indexing with a u64 index, type checking everywhere, arrays of arrays, arrays of structs, etc.

But there's still some work:

  • Parsing of arrays of aggregates is a bit lacking, it currently requires a[0][1] to be (a[0])[1].
  • The Index trait (or at least my_type.index(arg) for my_type[arg]) is invoked but not at all implemented yet.
  • No mutation at all. It doesn't compile if you try.
  • Code generation is a bit ordinary:
    • Unnecessary copies may be made during initialisation.
    • Element initialisation is done one by one, even when a MCP could do it all at once.
    • Every single index access is bounds checked (and will RVRT on failure) whether it needs it or not.
    • While testing large arrays the tests just ran out of gas. They're too expensive.

The first one could be fixed fairly easily, I just wanted to get on with it, working with Pest sucks.

The next two are non-trivial, we can tackle them when they're needed, which maybe now, IDK.

The code generation can be fixed with The Magic Of IR 🪄 -- as I was writing all the code gen code I was thinking, 'dammit, we're going to have to write all this again, except better'. But that's OK. The point of IR is to make that sort of thing relatively easy.

@adlerjohn
Copy link
Contributor

Could you not increase the gas used in tests? This is outside of consensus, so there's no real downside to putting an insane gas limit.

@otrho
Copy link
Contributor Author

otrho commented Nov 26, 2021

Could you not increase the gas used in tests? This is outside of consensus, so there's no real downside to putting an insane gas limit.

Yeah, though not on a per test basis, I think. But we could... I'm just saying that right now very large (MBs) arrays are very expensive, even just to instantiate. Rather than initialise every element individually for [val; count] arrays we could MCP from the data section. Or something.

@sezna
Copy link
Contributor

sezna commented Nov 28, 2021

Yeah that MCP optimization makes sense. But either way we can basically use infinite gas for tests right now if we'd like.

sezna
sezna previously approved these changes Nov 28, 2021
Copy link
Contributor

@sezna sezna left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, will re-approve after the latest version of master is merged in

Also add a new test using arrays and type parameters.
`unify()` takes a and b types, where b is the expected type (as per the
error message) and this was reversed for annotated expressions.
@otrho otrho merged commit 3fccd0c into master Nov 29, 2021
@otrho otrho deleted the otrho/18_arrays branch November 29, 2021 23:50
This was referenced Nov 30, 2021
@adlerjohn adlerjohn added compiler General compiler. Should eventually become more specific as the issue is triaged enhancement New feature or request labels Jan 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler General compiler. Should eventually become more specific as the issue is triaged enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants