-
-
Notifications
You must be signed in to change notification settings - Fork 37
[PoC] Parse integer data #32
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
base: main
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
import * as assert from 'assert'; | ||
import * as frontend from 'llparse-frontend'; | ||
|
||
import { Code } from '../code'; | ||
import { IRBasicBlock, IRValue } from '../compilation'; | ||
import { CONTAINER_KEY } from '../constants'; | ||
import { INodePosition, Node } from './base'; | ||
|
||
export class Int extends Node<frontend.node.Int> { | ||
protected doBuild(bb: IRBasicBlock, pos: INodePosition): void { | ||
const otherwise = this.ref.otherwise!; | ||
|
||
if (!otherwise.noAdvance) { | ||
bb = this.prologue(bb, pos); | ||
} | ||
|
||
this.tailTo(bb, otherwise, pos); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
import * as frontend from 'llparse-frontend'; | ||
|
||
import { Compilation } from '../compilation'; | ||
import { Node } from './base'; | ||
|
||
export class Int extends Node<frontend.node.Int> { | ||
public doBuild(out: string[]): void { | ||
|
||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,196 @@ | ||
import * as frontend from 'llparse-frontend'; | ||
|
||
import { Compilation } from '../compilation'; | ||
import { Node } from './base'; | ||
|
||
export class Int extends Node<frontend.node.Int> { | ||
public doBuild(out: string[]): void { | ||
this.prologue(out); | ||
|
||
switch (this.ref.bytes) { | ||
case 1: { | ||
if (this.ref.signed) { | ||
this.readInt8(out); | ||
} else { | ||
this.readUInt8(out); | ||
} | ||
break; | ||
} | ||
|
||
case 2: { | ||
if (this.ref.littleEndian) { | ||
this.readUInt16LE(out); | ||
} else { | ||
this.readUInt16BE(out); | ||
} | ||
break; | ||
} | ||
|
||
case 3: { | ||
if (this.ref.littleEndian) { | ||
this.readUInt24LE(out); | ||
} else { | ||
this.readUInt24BE(out); | ||
} | ||
break; | ||
} | ||
|
||
case 4: { | ||
if (this.ref.littleEndian) { | ||
this.readUInt32LE(out); | ||
} else { | ||
this.readUInt32BE(out); | ||
} | ||
break; | ||
} | ||
} | ||
|
||
this.tailTo(out, this.ref.otherwise!); | ||
} | ||
|
||
private readInt8(out: string[]) { | ||
const ctx = this.compilation; | ||
const index = ctx.stateField(this.ref.field); | ||
|
||
out.push(`${index} = (${ctx.bufArg()}[${ctx.offArg()}] & 2 ** 7) * 0x1fffffe;`) | ||
} | ||
|
||
private readUInt8(out: string[]) { | ||
const ctx = this.compilation; | ||
const index = ctx.stateField(this.ref.field); | ||
|
||
out.push(`${index} = ${ctx.bufArg()}[${ctx.offArg()}];`) | ||
} | ||
|
||
private readUInt16LE(out: string[]) { | ||
const ctx = this.compilation; | ||
const index = ctx.stateField(this.ref.field); | ||
|
||
switch (this.ref.byteOffset) { | ||
case 0: { | ||
out.push(`${index} = ${ctx.bufArg()}[${ctx.offArg()}];`) | ||
break; | ||
} | ||
|
||
case 1: { | ||
out.push(`${index} += ${ctx.bufArg()}[${ctx.offArg()}] * 2 ** 8;`); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nitpick: same here, let's use |
||
break; | ||
} | ||
} | ||
} | ||
|
||
private readUInt24LE(out: string[]) { | ||
const ctx = this.compilation; | ||
const index = ctx.stateField(this.ref.field); | ||
|
||
switch (this.ref.byteOffset) { | ||
case 0: { | ||
out.push(`${index} = ${ctx.bufArg()}[${ctx.offArg()}];`) | ||
break; | ||
} | ||
|
||
case 1: { | ||
out.push(`${index} += ${ctx.bufArg()}[${ctx.offArg()}] * 2 ** 8;`); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nitpick: ditto. |
||
break; | ||
} | ||
|
||
case 2: { | ||
out.push(`${index} += ${ctx.bufArg()}[${ctx.offArg()}] * 2 ** 16;`); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nitpick: ditto. |
||
break; | ||
} | ||
} | ||
} | ||
|
||
private readUInt32LE(out: string[]) { | ||
const ctx = this.compilation; | ||
const index = ctx.stateField(this.ref.field); | ||
|
||
switch (this.ref.byteOffset) { | ||
case 0: { | ||
out.push(`${index} = ${ctx.bufArg()}[${ctx.offArg()}];`) | ||
break; | ||
} | ||
|
||
case 1: { | ||
out.push(`${index} += ${ctx.bufArg()}[${ctx.offArg()}] * 2 ** 8;`); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nitpick: ditto. |
||
break; | ||
} | ||
|
||
case 2: { | ||
out.push(`${index} += ${ctx.bufArg()}[${ctx.offArg()}] * 2 ** 16;`); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nitpick: ditto. |
||
break; | ||
} | ||
|
||
case 3: { | ||
out.push(`${index} += ${ctx.bufArg()}[${ctx.offArg()}] * 2 ** 24;`); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nitpick: ditto. |
||
break; | ||
} | ||
} | ||
} | ||
|
||
private readUInt16BE(out: string[]) { | ||
const ctx = this.compilation; | ||
const index = ctx.stateField(this.ref.field); | ||
|
||
switch (this.ref.byteOffset) { | ||
case 0: { | ||
out.push(`${index} = ${ctx.bufArg()}[${ctx.offArg()}] * 2 ** 8;`); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nitpick: ditto. |
||
break; | ||
} | ||
|
||
case 1: { | ||
out.push(`${index} += ${ctx.bufArg()}[${ctx.offArg()}];`) | ||
break; | ||
} | ||
} | ||
} | ||
|
||
private readUInt24BE(out: string[]) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sounds like it could be generalized a bit. There is no runtime savings from having separate methods for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. My thinking was to keep these separated for readability - having all the logic in a single huge |
||
const ctx = this.compilation; | ||
const index = ctx.stateField(this.ref.field); | ||
|
||
switch (this.ref.byteOffset) { | ||
case 0: { | ||
out.push(`${index} = ${ctx.bufArg()}[${ctx.offArg()}] * 2 ** 16;`); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nitpick: ditto. |
||
break; | ||
} | ||
|
||
case 1: { | ||
out.push(`${index} += ${ctx.bufArg()}[${ctx.offArg()}] * 2 ** 8;`); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nitpick: ditto. |
||
break; | ||
} | ||
|
||
case 2: { | ||
out.push(`${index} += ${ctx.bufArg()}[${ctx.offArg()}];`) | ||
break; | ||
} | ||
} | ||
} | ||
|
||
private readUInt32BE(out: string[]) { | ||
const ctx = this.compilation; | ||
const index = ctx.stateField(this.ref.field); | ||
|
||
switch (this.ref.byteOffset) { | ||
case 0: { | ||
out.push(`${index} = ${ctx.bufArg()}[${ctx.offArg()}] * 2 ** 24;`); | ||
break; | ||
} | ||
|
||
case 1: { | ||
out.push(`${index} += ${ctx.bufArg()}[${ctx.offArg()}] * 2 ** 16;`); | ||
break; | ||
} | ||
|
||
case 2: { | ||
out.push(`${index} += ${ctx.bufArg()}[${ctx.offArg()}] * 2 ** 8;`); | ||
break; | ||
} | ||
|
||
case 3: { | ||
out.push(`${index} += ${ctx.bufArg()}[${ctx.offArg()}];`) | ||
break; | ||
} | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nitpick: Let's use
1 << 7
instead of2 ** 7
. Mixing binary and floating point operators looks fishy.I'm not sure that this line does what you want it to do, but I understand that this is work in progress. We can discuss it later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is based on how Node.js
Buffer
also implements reading integers: https://github.com/nodejs/node/blob/f5512ff61ecb668c2f49b7c05d3227ef7aa5e85f/lib/internal/buffer.js#L416I thought being consistent with core would make sense here? 🤔 I'm not sure what the pros or cons of switching to
1 << 7
would be. 🤷♂There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we could also just inline the result of the multiplication / shifting by changing this to:
What do you think?