Skip to content
This repository was archived by the owner on Mar 26, 2020. It is now read-only.

Property declaration support in interfaces#360

Open
ghost wants to merge 9 commits into
masterfrom
unknown repository
Open

Property declaration support in interfaces#360
ghost wants to merge 9 commits into
masterfrom
unknown repository

Conversation

@ghost

@ghost ghost commented Mar 22, 2018

Copy link
Copy Markdown

In an effort to continue the work in this PR #287, I've attempted to address the review comments and ensure the concerns have been addressed. Please review that original PR for more details.

Wherever possible I attempted to inject property-specific accessor information into the methods AST to share code generation. I was unable to do this at the parser level, since different languages have different needs.

@xianwen

xianwen commented May 14, 2018

Copy link
Copy Markdown

Hi, @MikeNachbaur:
I noticed you haven't signed the CLA yet, could you please sign it here: https://opensource.dropbox.com/cla/
Thanks a lot!

@xianwen xianwen self-requested a review July 17, 2018 19:01

@xianwen xianwen left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hi, @MikeNachbaur:
I noticed you haven't signed the CLA yet, could you please sign it here: https://opensource.dropbox.com/cla/
Thanks a lot!

try {
DJINNI_FUNCTION_PROLOGUE1(jniEnv, nativeRef);
const auto& ref = ::djinni::objectFromHandleAddress<::testsuite::PropertiesTestHelper>(nativeRef);
auto r = ref->other_method(::djinni::String::toCpp(jniEnv, j_argument));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Does r get modified? if not prefer to use const here.

@artwyman artwyman assigned ghost Apr 9, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants