Skip to content
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 CXX tutorial #1392

Merged
merged 16 commits into from
Nov 7, 2023
Merged

Add CXX tutorial #1392

merged 16 commits into from
Nov 7, 2023

Conversation

randomPoison
Copy link
Collaborator

@randomPoison randomPoison commented Oct 19, 2023

Add a number of slides that cover most of CXX's functionality and demonstrate how it can be used.

Fixes #823.

@randomPoison randomPoison requested a review from mgeisler October 19, 2023 21:21
@google-cla
Copy link

google-cla bot commented Oct 19, 2023

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@mgeisler
Copy link
Collaborator

Thanks @randomPoison for putting this up! I'm really excited about us getting small bite-sized slides on this since it's been a big hole in our coverage.

Before we merge this, we should think about how this part will work. First, cxx is not included in the playground, so mdbook test fails. This is similar to the Android, bare-metal, and concurrency chapters where the instructor has to copy-paste the relevant examples into a separate editor and run them there.

Next, will you be able to show the features of CXX using a running example? Or will we use different examples? On one hand, I like the realistic nature of the CXX tutorial with the blob store client, but I also feel that it's a bit too big of a project for this course. So I actually like the examples you came up with: the PlayingCard struct is small and easy to understand right away.

What I'm missing is more commentary and contrasting: can we (auto-)generate and show the corresponding C++ code for this? For Suit, you mention in the notes that CXX will generate a Rust struct, so I would love to see that as a student 🙂 I suggest including instructions on how to view this struct — both for the instructor and for people reading the course by themselves.

I would expand the page on the type mapping with notes on why the mapping is necessary. It's probably not immediately clear why a Rust String doesn't map 1:1 to std::string in C++.

Overall, I think this new chapter will be great, thanks for getting it started!

* Added separate slide explaining code generation for shared enums.
* Add additional speakers notes to better supplement existing slides.
@randomPoison
Copy link
Collaborator Author

@mgeisler

Before we merge this, we should think about how this part will work. First, cxx is not included in the playground, so mdbook test fails. This is similar to the Android, bare-metal, and concurrency chapters where the instructor has to copy-paste the relevant examples into a separate editor and run them there.

I wasn't really designing the code snippets to be run directly. They're mostly just partial demonstrations of the CXX syntax or the generated code, so there's not usually anything to run.

Next, will you be able to show the features of CXX using a running example?

I wasn't initially planning to create an example project, but Stephen expressed interest in adding one. It would also address the first point you brought up, since it would be an actually runnable example that I could pull up in class. I gave the blobstore example in the CXX repo a look, and I think (with some minor modification) it would be a good one to use as an example. I'll work on getting that integrated.

What I'm missing is more commentary and contrasting: can we (auto-)generate and show the corresponding C++ code for this?

Ah, I hadn't included more of the generated code because it's not really shown in the CXX tutorial, but I think I should be able to add some.

For Suit, you mention in the notes that CXX will generate a Rust struct, so I would love to see that as a student 🙂 I suggest including instructions on how to view this struct — both for the instructor and for people reading the course by themselves.

I've added a slide to demonstrate the code generated for a shared enum, and I've also added speakers notes to one of the earlier slides noting how to see the generated Rust and C++ code.

I would expand the page on the type mapping with notes on why the mapping is necessary. It's probably not immediately clear why a Rust String doesn't map 1:1 to std::string in C++.

There isn't really much to explain about why the type mappings are necessary: They're largely just defining a type in one language that corresponds to an existing type in the other language. Really the only confusing one is String vs std::string. I've added a speaker note elaborating on why these two can't be mapped to each other directly.

@randomPoison
Copy link
Collaborator Author

Oh, one other thing that I realized we need is a slide or two about using CXX in Android. There's CXX support in the Android build system, so we should include slides showing what that setup looks like, and make sure we also have an Android.bp for the runnable example project so that we can demonstrate building/running for Android.

@randomPoison
Copy link
Collaborator Author

@mgeisler I think I've addressed all outstanding feedback. Could you re-review now and let me know if I should make any further changes?

@fw-immunant This might also be a good point for you to come in and review it, now that I've gotten a pass of cleanup done.

@@ -0,0 +1,52 @@
# C++ Bridge Declarations

```rust,ignore
Copy link
Collaborator

Choose a reason for hiding this comment

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

As a more general tool, let us mark the untranslatable code snippets with

Suggested change
```rust,ignore
<!-- mdbook-xgettext: skip -->
```rust,ignore

This will completely avoid putting the code into the PO files and thus greatly reduce the workload of the translators.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Please add such a comment before every code block which cannot be translated, including the ones that include their content from another file.

You can see the effect of this with

cargo install mdbook-i18n-helpers
MDBOOK_OUTPUT='{"xgettext": {"pot-file": "before.pot"}}' mdbook build -d po
MDBOOK_OUTPUT='{"xgettext": {"pot-file": "after.pot"}}' mdbook build -d po
diff po/before.pot po/after.pot

You can then regenerate the POT file periodically to check that it only contains text that can be meaningfully translated.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Okay, can do! Which code blocks can't be translated, though? Do the translators usually translate any comments in code snippets?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Things are moving fast 😄 We've just gotten a PR which will help a lot with the translation effort by only extracting comments and strings: google/mdbook-i18n-helpers#109. This makes the large code blocks much less annoying for the translators.

To answer your questions:

Okay, can do! Which code blocks can't be translated, though?

Code where all static strings are things like "a: {a}" from a format string.

Do the translators usually translate any comments in code snippets?

It's a bit uneven today. The Simplified Chinese translation actually translated some comments: https://google.github.io/comprehensive-rust/zh-CN/hello-world/small-example.html (but not all, as far as I know).

Copy link
Collaborator

Choose a reason for hiding this comment

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

This makes the large code blocks much less annoying for the translators.

Meaning: don't worry too much about this now.

Results in (roughly) the following Rust:

```rust,ignore
#[repr(C)]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Very nice! During the class, I would want to generate this code and show the students where it is using bat or similar...

How do I do this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Probably the easiest way to view the generated code is to follow the speaker notes from the Bridge Module slide, i.e.:

  • To view the generated Rust code, use cargo-expand to view the expanded proc
    macro. For most of the examples you would use cargo expand ::ffi to expand
    just the ffi module (though this doesn't apply for Android projects).
  • To view the generated C++ code, look in target/cxxbridge.

You won't easily be able to do this directly with the code snippet in the slide, but you could pull up the blobstore example and build with Cargo to show this. The blobstore example is setup to build with both Cargo and Soong, so the easiest way to demo stuff will probably be with the Cargo version of the project.


Create a `rust_binary` that depends on `libcxx` and your `cc_library_static`.

```json
Copy link
Collaborator

Choose a reason for hiding this comment

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

I used javascript as the language in the Android part (should be mostly the same, so I'm just mentioning it for consistency).

I also used a particular format for referring to external files, see Rust Binaries.
I did that consistently in those slides: this was important since I hope people can copy-paste the code into their own slides and thus follow-along at home. To make my life easy, all the files are then actual live files so that I can navigate to the right directory and do m hello_rust_with_dep.

See build_all.sh for the script that I use to build everything once in a while. You should try to hook up your Android.bp files to the same script so that we can prevent bit-rot.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I just created #1434 to explain this process a bit — it's a little hacky, but I think it's better than nothing 😬

Copy link
Collaborator Author

@randomPoison randomPoison Oct 30, 2023

Choose a reason for hiding this comment

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

I also used a particular format for referring to external files, see Rust Binaries.

I'm not sure what you're referring to here? What "format" are you referring to? Do you mean the way that you're #includeing the snippets in from a larger example, whereas I'm writing most of them inline? Otherwise I'm not sure what we're doing differently.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure what you're referring to here? What "format" are you referring to?

Sorry, I was thinking of the

_filename.rs_:

```javascript

```

convention with the italic filename. If the precise name doesn't matter, then don't worry about following it (I found that the names matter a lot for the care of the AIDL code).

@mgeisler
Copy link
Collaborator

Hi @randomPoison, thanks for all the updates! I think it looks like a great improvement already!

I will let @fw-immunant read it through as well and then I think we should merge this soon.

Before we merge this, we should think about how this part will work. First, cxx is not included in the playground, so mdbook test fails. This is similar to the Android, bare-metal, and concurrency chapters where the instructor has to copy-paste the relevant examples into a separate editor and run them there.

I wasn't really designing the code snippets to be run directly. They're mostly just partial demonstrations of the CXX syntax or the generated code, so there's not usually anything to run.

When I'm teaching, I find it important to copy-paste the examples into a file and show the students that it can actually run. Doing this live is important since it encourages experimentation: someone will inevitably ask "How does X translate?" and then you can quickly try it out with the class.

The first question I have when I see a code generator like CXX is: where is the output? Can I see it? Can I understand it? Showing the students the actual code and going over the high-level structure would be important. I do the same when talking about AIDL: I show how you can find the output files and how they look.

Yes, people could do this copy-pasting themselves, but it helps anchor the information when you see it actually done. Otherwise it's too each to just skim over the slides.

I think we could spend 30-45 minutes on this part since it's important with all the interop that we need in Android. We could even spend longer if we could find a project that people could dive into here — the Android day is currently set to be half a day, but I would be happy if it was a full day course.

Next, will you be able to show the features of CXX using a running example?

I wasn't initially planning to create an example project, but Stephen expressed interest in adding one. It would also address the first point you brought up, since it would be an actually runnable example that I could pull up in class. I gave the blobstore example in the CXX repo a look, and I think (with some minor modification) it would be a good one to use as an example. I'll work on getting that integrated.

Thank you! I personally really like the blobstore tutorial: it's really well done because it shows integration in both directions. I also love that it's a real world example.

What I'm missing is more commentary and contrasting: can we (auto-)generate and show the corresponding C++ code for this?

Ah, I hadn't included more of the generated code because it's not really shown in the CXX tutorial, but I think I should be able to add some.

I think the examples you've added are great, they're bound to stir up a good discussion in class 😄

* Use javascript syntax highlighting for build files.
* Remove unused `BUCK` build file.
@randomPoison
Copy link
Collaborator Author

When I'm teaching, I find it important to copy-paste the examples into a file and show the students that it can actually run. Doing this live is important since it encourages experimentation: someone will inevitably ask "How does X translate?" and then you can quickly try it out with the class.

The first question I have when I see a code generator like CXX is: where is the output? Can I see it? Can I understand it? Showing the students the actual code and going over the high-level structure would be important. I do the same when talking about AIDL: I show how you can find the output files and how they look.

Yes, people could do this copy-pasting themselves, but it helps anchor the information when you see it actually done. Otherwise it's too each to just skim over the slides.

I think we could spend 30-45 minutes on this part since it's important with all the interop that we need in Android. We could even spend longer if we could find a project that people could dive into here — the Android day is currently set to be half a day, but I would be happy if it was a full day course.

This all sounds good, but I'm not sure what the actionable item is for this PR? I've tried to show the generated code as much as possible, and we now have a working example that we can demo to the class. Could you elaborate on what other changes you'd want to see so that it's easier to copy the sample code into a working example?

@mgeisler
Copy link
Collaborator

This all sounds good, but I'm not sure what the actionable item is for this PR? I've tried to show the generated code as much as possible, and we now have a working example that we can demo to the class. Could you elaborate on what other changes you'd want to see so that it's easier to copy the sample code into a working example?

You're right, my rambling was not super actionable!

I think we should merge what we have now — it's a huge improvement over the current course! You'll test it out in class in about two weeks and then we'll hopefully know more what works and what doesn't.

Thanks for landing this!

Copy link
Collaborator

@mgeisler mgeisler left a comment

Choose a reason for hiding this comment

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

Thanks!

@mgeisler
Copy link
Collaborator

@randomPoison and @fw-immunant, please coordinate with each other to see if Frances should also review this.

Copy link
Collaborator

@fw-immunant fw-immunant left a comment

Choose a reason for hiding this comment

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

LGTM, a couple minor comments inline.

src/android/interoperability/cpp/cpp-bridge.md Outdated Show resolved Hide resolved
src/android/interoperability/cpp/rust-result.md Outdated Show resolved Hide resolved
@mgeisler
Copy link
Collaborator

mgeisler commented Nov 5, 2023

Hi @randomPoison, please resolve the last discussions with @fw-immunant and merge this.

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.

Add examples of how to use CXX
3 participants