Skip to content

src: pack struct by sorting fields#36

Open
fanatid wants to merge 2 commits into
nodejs:mainfrom
fanatid:fields-order
Open

src: pack struct by sorting fields#36
fanatid wants to merge 2 commits into
nodejs:mainfrom
fanatid:fields-order

Conversation

@fanatid

@fanatid fanatid commented Jan 11, 2020

Copy link
Copy Markdown
Contributor

Issue #35
Decide just drop code, interesting thing.

Fields in struct same, but by some cases errors on tests. Currently do not know llparse too good for say why 🤔

btw, should not i8/i16/i32/i64 be u8/u16/u32/u64?

@indutny indutny left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM in general, but it has to cover bitcode as well! C headers apply both to C and bitcode output.

@fanatid

fanatid commented Jan 14, 2020

Copy link
Copy Markdown
Contributor Author

Sorry, I do not think that I understand. I should change something in bitcode for fixing tests?

@indutny

indutny commented Jan 14, 2020

Copy link
Copy Markdown
Member

Not in the tests. The order of fields has to be changed in the bitcode generator.

@fanatid

fanatid commented Jan 14, 2020

Copy link
Copy Markdown
Contributor Author

Thank you.
I see that I also need change fields in bitcode / js:

this.state.addField(constants.TYPE_INDEX, constants.STATE_INDEX);

out.push(` ${ctx.indexField()} = 0;`);

@indutny

indutny commented Jan 15, 2020

Copy link
Copy Markdown
Member

Well, maybe not in JS, but you got it right!

@fanatid

fanatid commented Jan 20, 2020

Copy link
Copy Markdown
Contributor Author

I fixed code for bitcode.

@DemiMarie

Copy link
Copy Markdown

Now bitcode is removed!

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.

3 participants