-
Notifications
You must be signed in to change notification settings - Fork 193
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
Fix getOrInsertFunction syntax error for latest LLVM #14
Conversation
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 looks great; thank you! I have a couple of style suggestions inline.
skeleton/Skeleton.cpp
Outdated
); | ||
std::vector<Type*> paramsType = {Type::getInt32Ty(Ctx)}; | ||
Type *retType = Type::getVoidTy(Ctx); | ||
FunctionType *logFuncType = FunctionType::get(retType,paramsType,false); |
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.
Can you please use spaces between the commas in function calls?
skeleton/Skeleton.cpp
Outdated
Constant *logFunc = F.getParent()->getOrInsertFunction( | ||
"logop", Type::getVoidTy(Ctx), Type::getInt32Ty(Ctx), NULL | ||
); | ||
std::vector<Type*> paramsType = {Type::getInt32Ty(Ctx)}; |
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.
A better name here is probably paramTypes
, since there are multiple types. (The current name implies that it’s a single type.)
With this change, does everything seem to work with the latest LLVM? (Including the linker error from #12?) If so, we should probably update the README. |
Yeah, thanks for the advice. I'll fix the coding style. All the examples works fine with LLVM6(the latest svn trunk), so we could update the README too.
|
Awesome; thank you! |
A simple fix for issue #13