-
Notifications
You must be signed in to change notification settings - Fork 61
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
Provided the support of shared pointer with example #57
Conversation
Signed-off-by: ossdev <[email protected]>
Hi @gnodet Can you please look into the changes, and let us know if anything else is required from our side. Seeing the package supporting the shared pointer would be a delight. |
@gnodet Is there any update regarding the progress of this activity? |
@rhenwood-arm We added the support in a hawtJNI generator to parse this field and generate the corresponding JNI. To test this flag we have defined a variable If you have any more queries, let me know. I will be more than to happy to explain it. |
Thanks @ossdev07 One further question: what are the steps I need to go through to test this new functionality on a AArch64 machine? |
@rhenwood-arm You just need to install all the dependencies and test the package We can achieve it by the command By this command, all the dependencies get to install, test cases get to run and we could see the result on console. |
I'll try to review the PR this week. |
Thanks @ossdev07. The self-test passes in my Java AArch64 container:
|
@gnodet : have you had a chance to review this PR? |
@gnodet Any update? |
hawtjni-generator/src/main/java/org/fusesource/hawtjni/generator/StructsGenerator.java
Outdated
Show resolved
Hide resolved
hawtjni-generator/src/main/java/org/fusesource/hawtjni/generator/StructsGenerator.java
Outdated
Show resolved
Hide resolved
hawtjni-generator/src/main/java/org/fusesource/hawtjni/generator/model/ReflectField.java
Outdated
Show resolved
Hide resolved
Any update on the comments ? I'd like to get rid of this PR. @ossdev07 |
@gnodet : have you had a chance to review this PR? |
It does not seem to work for me:
|
Also, the getter for the shared pointer is an |
@gnodet |
@ossdev07 this looks almost good to me but the example still fails to compile:
I think the return value needs to be changed to something else:
|
In our setup it is always pass , can you tell me how are you testing means command |
Just running Here's a more complete log:
|
I checked in 3 different systems and its getting passed , i think it's because of some compilation settings, my systems has default configurations of compilation flags .. |
I found one difference in your setup is "--with-universal" flag in configure command .. |
My C++ compiler seems to be more strict. Any way you could improve the example to not return 0, but use real getters / setters to access a shared pointer ? That would give me some confidence that this thing is actually working... also, if that's still related to fusesource/rocksdbjni#10 and if that's the only driver for supporting shared pointers, you may really want to investigate the RocksDB JNI layer provided by rocksdb directly. |
… and resolved review comment Signed-off-by: ossdev07 <[email protected]>
Hi @gnodet We have made changes as per your review comments and now we are returning non-zero value (i.e. shared_ptr) Please refer to latest commit at this pull request 57 |
@gnodet it's long pending PR almost chasing for last 1 year, Please have a look on modifications. |
@gnodet require your further review for this change.
@gnodet require your further review for this change. |
@gnodet , Require your review and valuable suggestions. Thanks. |
After having fixed a bunch of issues in the example (see https://github.com/gnodet/hawtjni/commits/ossdev07-shared_ptr_support), the compiler still does not build: there's a conversion error in the generated native code:
|
Hi @gnodet Thanks for your review and build attempt. I tried our branch and your branch, here both reported success for command If you think, this could be due to any environmental change, then please suggest me how can I reproduce the same... Could this build difference be due to compiler version/flag difference (like using more strict/advance compiler) ? |
I'm on OSX. I've been able to build the branch by switching to a recent gcc version instead of using the one that comes by default with OSX. |
Well, I can't make it succeed again :-( I'll investigate. |
Hi @gnodet If review of my system environment info suggests anything regarding this:
I have not removed non-required info as well. Thanks. |
on Mac, I'm getting the same error as @gnodet and really can't understand why this would have been closed. What was the solution? |
I think I had to upgrade Xcode iirc. |
As per the conversation in the PR #51 , provided the support of shared pointer along with an example.
Please consider the PR for the merge.