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, bug fixes and improvements #148

Merged
merged 21 commits into from
Oct 6, 2018
Merged

Static arrays, bug fixes and improvements #148

merged 21 commits into from
Oct 6, 2018

Conversation

ballercat
Copy link
Owner

@ballercat ballercat commented Sep 30, 2018

Quality of life improvements and bug fixes

New Features

Implements static arrays as opaque types, when defined on a global scope:

// The variable 'x' below has a custom type (not compiled) with a fixed size
const x: i32[] = [1, 2, 'a', 'A', 5];

// This is different from an array which is just a "view" into memory regions
const z: i32[] = 0;

export function test(): i32 {
  // static arrays have a fixed size (in bytes)
  const sz: i32 = sizeof(x); // 20 == sizeof(i32) * 5
  const szi: i32 = sizeof(z); // 4 === sizeof(i32) or the size of the pointer value

  // static arrays can be accessed with static keys
  const y: i32 = x[3]; // numerical value of 'A'

  // dynamic keys are also possible
  const w: i32[] = x; // here we assign an array view to start at the static address
  let i: i32 = 2;
  return w[i]; // numerical value of 'a'
}

These arrays are not possible to define inside a function call, so they are statics. This is a pretty decent metod of witing Data section entries into a binary, which was pretty difficult to do before.

Fixes

  • walt-cli should be using peerDeps for compiler/linker
  • walt-cli compiler api update to work with walt-compiler@0.10.0+
  • walt-cli did not use working directory
  • walt-compiler did not handle escape sequences correctly
  • walt-cli add additional tests

Additions

Will implement #79

  • memory.dataSize()
  • memory.grow()
  • memory.size()

@coveralls
Copy link

coveralls commented Sep 30, 2018

Pull Request Test Coverage Report for Build 162

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.004%) to 99.36%

Totals Coverage Status
Change from base Build 150: 0.004%
Covered Lines: 1893
Relevant Lines: 1893

💛 - Coveralls

@JobLeonard
Copy link

const sz: i32 = sizeof(x); // 20 == sizeof(i32) * 5
const szi: i32 = sizeof(z); // 4 === sizeof(i32) or the size of the pointer value

I think it would be better if we avoid overloading sizeof like that, to make better use of the type information available.

Say that at first I think I need a fixed size array for some purpose, but later change my mind, and deep down in my refactored code I have a sizeof.

Or even simpler: I just mix up the two while writing the code. It happens!

If we have two function names instead, for example size and sizeof. size only works on static arrays (and perhaps static strings, I guess), while sizeof only works on non-static pointers. In that scenario, accidentally calling sizeof on x or size on z will give a compile time error, catching the bug before it happens.

Arty Buldauskas added 7 commits October 1, 2018 12:57
@ballercat
Copy link
Owner Author

@JobLeonard I hear you, but I don't agree this will lead to negative trade-offs. I been thinking about it for a while, so it took me a bit to respond.

The reason why sizeof() is able to give us the correct size at all is because the static array is using a hidden struct type behind the scenes. By hidden I mean it's not referencable, is only used by the compiler to perform field lookups and isn't compiled into the runtime.

To illustrate

type ArrayLike = { 0: i32, 1: i32 };
const x: ArrayLike = <some-pointer-value>;
sizeof(x); // The type of x is known to be "ArrayLike" and its size is 2 * sizeof(i32)

const y: i32[] = [1, 2];  // hidden static struct type here, same keys as "ArrayLike"
sizeof(y); // The type of x is known and it's size is 2 * sizeof(i32)

So the behavior is equivalent to existing sizeof rules, but because the type is not a symbol the only way to get the correct sizeof result is to reference y directly. Which is nice, because what we ask for with the sizeof call above is "what is the size of this data section?". Otherwise it will look like any other i32 and be sizeof(i32) which is 4.

This behavior has a side-effect where functions sharing this reference are required to pass in the length of the static data section manually to other functions to make it useful. Here is an example in action

https://github.com/ballercat/tiny-mark/blob/8a191cf8d7f4ba6fdfc365b80b2b56d6bd9a7555/src/walt/index.walt#L60-L65

@ballercat ballercat merged commit 13f4d86 into master Oct 6, 2018
@JobLeonard
Copy link

I been thinking about it for a while, so it took me a bit to respond.

That is the healthier attitude to internet discussions, I'm grateful you gave it some thought! :)

I don't think we're quite reasoning about the same thing though?

Your line of thought: does sizeof behaviour make consistent logical sense?

My line of thought: does overloading a keyword make it more or less likely that bugs appear due to this overloading?

Your reasoning is sound, but the examples I gave have nothing to do with it, and everything to do with how people rewrite their code.

Of course, it still comes down to a decision which trade-off matters more even then.

@JobLeonard
Copy link

/ Constants
const HEADER: i32 = 0x00;
const H1_OPEN: i32[] = ['<', 'h', '1', '>'];
const H1_CLOSE: i32[] = ['<', '/', 'h', '1', '>'];
const UL_OPEN: i32 = ['<', 'u', 'l', '>'];
const UL_CLOSE: i32 = ['<', '/', 'u', 'l', '>'];
const LI_OPEN: i32 = ['<', 'l', 'i', '>'];
const LI_CLOSE: i32 = ['<', '/', 'l', 'i', '>'];

Wait, why are some of these i32[], and some i32 without brackets?

@ballercat
Copy link
Owner Author

Oh hey, good catch!

The grammar rules were too permissive and allowed for non-array types in the declarations. Simple enough to fix in #150

@ballercat ballercat deleted the statics branch October 8, 2018 18:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants