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

Feature: ConstantInt - support get from string #75 #76

Merged
merged 5 commits into from
Jan 3, 2019

Conversation

ovr
Copy link
Contributor

@ovr ovr commented Dec 14, 2018

Signed-off-by: Dmitry Patsura talk@dmtry.me

Signed-off-by: Dmitry Patsura <talk@dmtry.me>
Signed-off-by: Dmitry Patsura <talk@dmtry.me>
llvm-node.d.ts Outdated Show resolved Hide resolved
Copy link
Owner

@MichaReiser MichaReiser left a comment

Choose a reason for hiding this comment

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

Sorry that it took me so long to respond. It looks good to me. I just added two comments about possible extensions we could add.

Thank you once more for this PR

@@ -199,6 +199,8 @@ declare namespace llvm {
private constructor();

readonly value: number;
Copy link
Owner

Choose a reason for hiding this comment

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

Should we add a check to get that throws if the value is larger than what JS supports?

src/ir/constant-int.cc Outdated Show resolved Hide resolved
@MichaReiser MichaReiser merged commit 19ce472 into MichaReiser:master Jan 3, 2019
@ovr ovr deleted the constant-int-from-string branch January 3, 2019 09:17
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

2 participants