ParseInt Implementation

[MaxGraey]

Hi dcode! I'm trying to help you with std functions.
There is parseInt function. I tested this part on pure typescript and it's behave exactly the same as original JS method. For real tests in AS environment need some parser functionality (String literals) which missing yet.

TODO

1. Allow parsing binary & octal numeric strings with prefixes 0o, 0b? (But original Javascript method can't do that)
2. Write tests

[dcodeIO]

The tokenizer also has this one derived from tsc. Might make sense to bundle src/util/charcode with stdlib as well so there's a source for the constants. Otherwise, declaring a const local is actually handled as a constant (doesn't result in an actual wasm local, unlike a const global that stills becomes a global for linking but all its uses are substituted), but must be initialized with the exact value (can't use charCodeAt here).

[dcodeIO]

One way to test it, btw., could be to use the charcodes (u16) as the module's memory and then load<u16> instead of charCodeAt for testing.

[MaxGraey]

Got it. May be tomorrow I switch charCodeAt to load<u16> and will write some tests.

[MaxGraey]

One way to test it, btw., could be to use the charcodes (u16) as the module's memory and then load instead of charCodeAt for testing.

Unfortunately I can't do that because ptr member in String is private =(

[dcodeIO]

Yeah, String cannot be used at all yet. But, if the module's memory would contain the string's u16 char codes starting at offset 0 with its length known, ptr would simply be 0 and length a known value.

That doesn't lead to something that "just works", of course, it's just the only test case possible atm.

[MaxGraey]

Hmm, sounds promising

[dcodeIO]

Allow parsing binary & octal strings? (But original Javascript method can't do that)

Just tested in node.js:

> parseInt("0011", 2)
3
> parseInt("0011", 8)
9

[MaxGraey]

Right, current implementation support any radix from 2 to 36 the same as native JS method.
I mean this misalignment with prefixes:

> parseInt("0x10") // expecting 16
16
> parseInt("0o10") // expecting 8
0
> parseInt("0b10") // expecting 2
0

[MaxGraey]

Hmm, after I added additional file in test directory script npm run test:config became failure.

[dcodeIO]

Yeah, config is tested against tsc, and it doesn't like std/assembly/string.ts(211,1): error TS1206: Decorators are not valid here..

Workaround I have in mind currently is to implicitly make all exports from stdlib files globals so functions don't need a @global decorator.

[MaxGraey]

I don't think so problem in let in this case. But following you description I reverted this changes

[MaxGraey]

It seems problem in new changes in std Heap. Can you check?

[dcodeIO]

The other differences in the the latest log are expected as well because there have been changes to other parts of the standard library meanwhile. For example the std:heap/Heap class doesn't exist anymore in this form, but directly implements allocate_memory as functions.

You can regenerate the fixtures by running npm run test:compiler -- --create.

[MaxGraey]

So can I somehow fix that?

[dcodeIO]

Yes, just regenerate the outdated fixture your code is compared to using the command above. Fixtures are there to visually confirm that the output of a known-to-be-good test didn't change due to a possibly unrelated change. Hence, when editing code that affects a fixture, the intended procedure is to regenerate the fixture and then manually validate, by means of looking at the diff, that the changes are expected.

[MaxGraey]

Got it! It's kind of snapshotting

[dcodeIO]

Wouldn't this be sufficiently covered by a radix: i32 = 10 parameter above?

[MaxGraey]

These are fully justified changes. You can try in js:

console.log(parseInt('10')) // => 10
console.log(parseInt('10', 10)) // => 10

It's clear. But what about specified radix with hex prefix?

console.log(parseInt('0x10', 10)) // => 0
console.log(parseInt('0x10')) // => 16
console.log(parseInt('0x10', 0)) // => 16

So radix with zero or unspecified value mean actually auto radix

[dcodeIO]

I see, makes sense.

[MaxGraey]

I added tests for parseInt. Only one case produce unreachable issue. Need more investigation

[MaxGraey]

In this case we get unreachable

[dcodeIO]

Hmm, that's strange. Probably hitting an assertion somewhere then. Might make sense to hook assert up to an import on failure now, so we know where that is :)

[MaxGraey]

It always print:

Testing compiler/std/string.ts
parse incl. I/O: 4.978ms / compile: 4.763ms
validate OK
[trap unreachable]
interpret ERROR:undefined
Created
Created optimized

And I can't figure out what is problem

[dcodeIO]

To debug this further, you can now import an abort function that receives all the information necessary.

@global
declare function abort(message: string | null, filename: string, line: i32, column: i32): void;

...

On the JS side, that'd look like:

function abort(msg, file, line, column) {
  throw Error("Assertion failed: '" + (msg ? getString(msg) : "") + "' at " + getString(file) + ":" + line + ":" + column);
}

function getString(ptr) {
  var len = new Uint32Array(exports.memory.buffer, ptr)[0];
  var str = new Uint16Array(exports.memory.buffer, ptr + 4).subarray(0, len);
  return String.fromCharCode.apply(String, str);
}

with msg and file pointing at the string in memory, that is a 32-bit length N + N * 16-bit charcodes.

[dcodeIO]

I created something above that might help as a starting point. It possibly doesn't catch all the corner cases yet and has only rudimentary tests. Most important difference is that it outputs i64s, not sure how far we want to go with compatibility there.

[dcodeIO]

How do you like to proceed with this one? Was probably a bit insensible of me to implement something on my own though you were working on something already, sorry for that.

[MaxGraey]

It's ok, you already implement this so I close this