Skip to content

Add ConstantExpr class#42

Closed
vihanb wants to merge 3 commits intoMichaReiser:masterfrom
vsl-lang:add-constant-expr
Closed

Add ConstantExpr class#42
vihanb wants to merge 3 commits intoMichaReiser:masterfrom
vsl-lang:add-constant-expr

Conversation

@vihanb
Copy link
Contributor

@vihanb vihanb commented Mar 2, 2018

This adds a ConstantExpr class with a few methods, doesn't have tests or most methods added, but I'm making the PR as I was wondering if there's a way to avoid so much duplication of code. Most methods have basically identical code except for the method of llvm::ConstantExpr called.

Signed-off-by: Vihan contact@vihan.org

vihanb added 3 commits March 1, 2018 18:48
Signed-off-by: Vihan <contact@vihan.org>
Signed-off-by: Vihan <contact@vihan.org>
Signed-off-by: Vihan <contact@vihan.org>
@MichaReiser
Copy link
Owner

Thanks @vihanb for your pull request.

If everything is identical for a group of methods except the actual method called, you can make use of templates like I did in IRBuilder.

Except that, no, writing a NAN module is quite repetetive. I hope to be able to reduce the boilerplate with migrating to N-API. However, there is currently an impediment because there seems to be no way to correctly model Inheritance.

}

v8::Local<v8::Object> ConstantExprWrapper::of(llvm::ConstantExpr* constantExpr) {
auto constructor = Nan::GetFunction(Nan::New(constantExprTemplate())).ToLocalChecked();
Copy link
Owner

Choose a reason for hiding this comment

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

Please also add ConstantExpr to the ConstantWrapper::of operation. This ensures that instances of ConstantExpr are returned as such.

Copy link
Contributor

Choose a reason for hiding this comment

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

ping @vihanb

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