-
Notifications
You must be signed in to change notification settings - Fork 95
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
Add methods for LLVMTargetDataRef #141
Conversation
…methods, better align DataLayout(was LlvmTargetdata) with c++ api method names
I've made some changes in line with the feedback, hopefully I'm getting closer. I went for option 2 and created a
So in order to keep the name in line with the c name, I just dropped the duplicate. |
Overall this LGTM now, minus the naming nits on methods. |
Co-authored-by: Tanner Gooding <tagoo@outlook.com>
Co-authored-by: Tanner Gooding <tagoo@outlook.com>
Thanks! |
Just wanted to make sure you saw (#141 (comment)):
I didn't explicitly provide a suggestion for all the names that should be updated and it looks like a couple still remain. https://llvm.org/doxygen/classllvm_1_1DataLayout.html has the C++ names if you need a reference. |
Thanks, I'll look at the others. |
Thanks! |
This adds methods to
LLVMTargetRef
for some of the underlying LLVM api calls, covers the ones I want, struct element offsets, and others that I saw that took aLLVMTargetRef
as the first parameter. I also added a class that wraps theLLVMTargetRef
struct,LLVMTargetData
thinking of #140 but not sure I've understood that right as doesn't seem to add much so let me know if that should come out. Added some simple tests for the new methods inLLVMTargetRef
to at least make sure they return something sensible.