Intelligently convert C/C++ comments to Rust #791

dylanmckay · 2017-07-06T00:32:58Z

With this change, we can correctly parse C++ block comments.

/**
 * Does a thing
 *
 * More documentation. This test does something
 * useful.
 */

into

/// Does a thing
///
/// More documentation. This test does something
/// useful.

Fixes #426.

dylanmckay · 2017-07-06T00:41:57Z

Note that this does not implement the ///< doxygen syntax (which causes the doc comment to apply to the previous item).

As far as I can tell, this is currently already broken and is causing us to emit the wrong doc comments off by one on struct fields/enum variants which use the syntax.

dylanmckay · 2017-07-06T00:49:20Z

I need to modify this so that it also treats regular comments (//) as doc comments, will do so later today.

emilio · 2017-07-06T06:36:14Z

This is lovely, thank you!

Could we also add the test-case from #426?

dylanmckay · 2017-07-06T07:50:52Z

I can do that.

Also, I'm going to refactor this so that rather than doing the C++ -> Rust comment conversion at the codegen level, we do it when building the AST. As far as I understand it, that would be a more appropriate place and (depending on how we build the AST - I haven't looked at it yet), we may be able to have a single call which converts comments rather than having multiple over the place.

emilio · 2017-07-06T08:34:56Z

Yeah, I think that's fine, though if it gets nasty we could also move the preprocess call to attributes::doc, and use it where it's not currently used. That'd remove the duplication too.

dylanmckay · 2017-07-06T10:09:57Z

I've added the test case from #426 and also moved the preprocessing to the IR stage.

Let me know if you'd like me to change anything in order to merge :)

emilio

I'm thinking that handling comment processing in code generation makes sense, actually, because we can re-indent depending on the nesting level of the item, etc... However, that's followup work in any case, and it's easy enough to move it so... This can land with r=me with Travis fixed:

error: missing documentation for a module
 --> src/ir/mod.rs:8:1
  |
8 | pub mod comment;
  | ^^^^^^^^^^^^^^^^
  |

emilio · 2017-07-06T11:48:47Z

Thanks again for doing this btw! :)

dylanmckay · 2017-07-06T12:56:39Z

Thanks again for doing this btw! :)

No problem, thanks for working on the project :)

I've fixed the comment issue but I can't get the convert-cpp-comments-to-rust test I added to match the expected header file.

Here is a part of the diff from the travis run

It seems as if it's related to environment differences between my computer and CI, but I can't really replicate the exact same build setup as them.

pub const _STDINT_H: ::std::os::raw::c_uint = 1;
 pub const _FEATURES_H: ::std::os::raw::c_uint = 1;
+pub const __USE_ANSI: ::std::os::raw::c_uint = 1;
 pub const _ISOC95_SOURCE: ::std::os::raw::c_uint = 1;
 pub const _ISOC99_SOURCE: ::std::os::raw::c_uint = 1;
-pub const _ISOC11_SOURCE: ::std::os::raw::c_uint = 1;
 pub const _POSIX_SOURCE: ::std::os::raw::c_uint = 1;
 pub const _POSIX_C_SOURCE: ::std::os::raw::c_uint = 200809;
 pub const _XOPEN_SOURCE: ::std::os::raw::c_uint = 700;
 pub const _XOPEN_SOURCE_EXTENDED: ::std::os::raw::c_uint = 1;
 pub const _LARGEFILE64_SOURCE: ::std::os::raw::c_uint = 1;
-pub const _DEFAULT_SOURCE: ::std::os::raw::c_uint = 1;
+pub const _BSD_SOURCE: ::std::os::raw::c_uint = 1;
+pub const _SVID_SOURCE: ::std::os::raw::c_uint = 1;
 pub const _ATFILE_SOURCE: ::std::os::raw::c_uint = 1;
-pub const __USE_ISOC11: ::std::os::raw::c_uint = 1;
 pub const __USE_ISOC99: ::std::os::raw::c_uint = 1;
 pub const __USE_ISOC95: ::std::os::raw::c_uint = 1;
 pub const __USE_POSIX: ::std::os::raw::c_uint = 1;
 pub const __USE_POSIX2: ::std::os::raw::c_uint = 1;
 pub const __USE_POSIX199309: ::std::os::raw::c_uint = 1;
 pub const __USE_POSIX199506: ::std::os::raw::c_uint = 1;
 pub const __USE_XOPEN2K: ::std::os::raw::c_uint = 1;
 pub const __USE_XOPEN2K8: ::std::os::raw::c_uint = 1;
 pub const __USE_XOPEN: ::std::os::raw::c_uint = 1;
 pub const __USE_XOPEN_EXTENDED: ::std::os::raw::c_uint = 1;
 pub const __USE_UNIX98: ::std::os::raw::c_uint = 1;
 pub const _LARGEFILE_SOURCE: ::std::os::raw::c_uint = 1;
 pub const __USE_XOPEN2K8XSI: ::std::os::raw::c_uint = 1;
 pub const __USE_XOPEN2KXSI: ::std::os::raw::c_uint = 1;
 pub const __USE_LARGEFILE: ::std::os::raw::c_uint = 1;
 pub const __USE_LARGEFILE64: ::std::os::raw::c_uint = 1;
 pub const __USE_MISC: ::std::os::raw::c_uint = 1;
+pub const __USE_BSD: ::std::os::raw::c_uint = 1;
+pub const __USE_SVID: ::std::os::raw::c_uint = 1;

Does this ring any bells @emilio?

emilio · 2017-07-07T17:14:52Z

tests/headers/convert-cpp-comment-to-rust.hpp

@@ -0,0 +1,16 @@
+#include <stddef.h>
+#include <stdint.h>


Yeah, tests can't have system includes to pass, since these are machine-dependent.

emilio · 2017-07-07T17:15:34Z

tests/headers/convert-cpp-comment-to-rust.hpp

+#include <stddef.h>
+#include <stdint.h>
+
+typedef uint32_t mbedtls_mpi_uint;


instead you can remove them and, let's say, replace uint32_t for unsigned, and size_t for unsigned long. they're irrelevant to the test-case anyway, so it seems sensible.

Very helpful, thanks!

With this change, we can correctly parse C++ block comments. ``` /** * Does a thing * * More documentation. This test does something * useful. */ ``` into ``` /// Does a thing /// /// More documentation. This test does something /// useful. ``` Fixes rust-lang#426.

dylanmckay · 2017-07-08T00:31:29Z

Tests are now green @emilio, thanks for the help!

emilio · 2017-07-08T16:30:08Z

Looks great, thanks again for this!

@bors-servo r+

bors-servo · 2017-07-08T16:30:09Z

📌 Commit 239a015 has been approved by emilio

bors-servo · 2017-07-08T16:30:11Z

⌛ Testing commit 239a015 with merge f19e576...

Intelligently convert C/C++ comments to Rust With this change, we can correctly parse C++ block comments. ```cpp /** * Does a thing * * More documentation. This test does something * useful. */ ``` into ```rust /// Does a thing /// /// More documentation. This test does something /// useful. ``` Fixes #426.

bors-servo · 2017-07-08T17:03:40Z

☀️ Test successful - status-travis
Approved by: emilio
Pushing f19e576 to master...

jethrogb · 2017-07-08T17:47:54Z

Author of #426 here. I didn't get a notification when this PR was opened.

Why are you converting into Rust doc comments? It seems highly unlikely that rustdoc or rustdoc --test will be able to deal with random C++ comments correctly.

emilio · 2017-07-08T20:01:19Z

Why are you converting into Rust doc comments? It seems highly unlikely that rustdoc or rustdoc --test will be able to deal with random C++ comments correctly.

Well, sure, but that's followup work anyway. I think it's nice to have the comment in a processable format, which is what this PR enables. This PR also fixes the most common cases where people doesn't use markdown for their C/C++ docs.

If we want to support fully rustdoc, etc, there may be more followup work needed. An easy way to generate comments and make rustdoc not complain is making comments normal // comments.

Anyway, I've followed up in #799 with fixes and a few features that this PR enabled. I think Markdown/Doxygen support can be discussed in a new issue. Will file one to that effect.

emilio · 2017-07-08T20:09:06Z

I opened #800 for that.

dylanmckay · 2017-07-08T23:05:45Z

@emilio would it be possible for a new revision to be published so I can use this in one of my own crates?

emilio · 2017-07-09T01:47:20Z

@dylanmckay sure, though I'd wait for #799 to merge to fix the panic when a class is documented with an empty multiline comment (/**/), I don't want to release known panics. I'll push a version bump to that PR :)

dylanmckay · 2017-07-09T04:13:10Z

Sounds good to me, thanks @emilio :)

highfive added the S-awaiting-review label Jul 6, 2017

dylanmckay force-pushed the preprocess-doc-comments branch from a68f9a1 to 37f5d85 Compare July 6, 2017 00:39

dylanmckay changed the title ~~Intelligently convert C/C++ comments to Rust~~ WIP: Intelligently convert C/C++ comments to Rust Jul 6, 2017

dylanmckay force-pushed the preprocess-doc-comments branch 2 times, most recently from 78d2166 to 233f5db Compare July 6, 2017 10:06

dylanmckay changed the title ~~WIP: Intelligently convert C/C++ comments to Rust~~ Intelligently convert C/C++ comments to Rust Jul 6, 2017

dylanmckay force-pushed the preprocess-doc-comments branch 2 times, most recently from ae79118 to aefffc6 Compare July 6, 2017 10:22

emilio approved these changes Jul 6, 2017

View reviewed changes

dylanmckay force-pushed the preprocess-doc-comments branch 2 times, most recently from bc15759 to 4dca1b1 Compare July 6, 2017 12:38

emilio reviewed Jul 7, 2017

View reviewed changes

dylanmckay force-pushed the preprocess-doc-comments branch from 4dca1b1 to 239a015 Compare July 7, 2017 23:43

highfive assigned emilio Jul 8, 2017

highfive added S-awaiting-merge and removed S-awaiting-review labels Jul 8, 2017

bors-servo merged commit 239a015 into rust-lang:master Jul 8, 2017

highfive removed the S-awaiting-merge label Jul 8, 2017

emilio mentioned this pull request Jul 8, 2017

Support markdown in C++ doc comments. #800

Closed

dylanmckay deleted the preprocess-doc-comments branch July 8, 2017 23:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Intelligently convert C/C++ comments to Rust #791

Intelligently convert C/C++ comments to Rust #791

dylanmckay commented Jul 6, 2017

dylanmckay commented Jul 6, 2017

dylanmckay commented Jul 6, 2017

emilio commented Jul 6, 2017

dylanmckay commented Jul 6, 2017

emilio commented Jul 6, 2017

dylanmckay commented Jul 6, 2017

emilio left a comment

emilio commented Jul 6, 2017

dylanmckay commented Jul 6, 2017

emilio Jul 7, 2017

emilio Jul 7, 2017

dylanmckay Jul 7, 2017

dylanmckay commented Jul 8, 2017

emilio commented Jul 8, 2017

bors-servo commented Jul 8, 2017

bors-servo commented Jul 8, 2017

bors-servo commented Jul 8, 2017

jethrogb commented Jul 8, 2017

emilio commented Jul 8, 2017

emilio commented Jul 8, 2017

dylanmckay commented Jul 8, 2017

emilio commented Jul 9, 2017

dylanmckay commented Jul 9, 2017

Intelligently convert C/C++ comments to Rust #791

Intelligently convert C/C++ comments to Rust #791

Conversation

dylanmckay commented Jul 6, 2017

dylanmckay commented Jul 6, 2017

dylanmckay commented Jul 6, 2017

emilio commented Jul 6, 2017

dylanmckay commented Jul 6, 2017

emilio commented Jul 6, 2017

dylanmckay commented Jul 6, 2017

emilio left a comment

Choose a reason for hiding this comment

emilio commented Jul 6, 2017

dylanmckay commented Jul 6, 2017

emilio Jul 7, 2017

Choose a reason for hiding this comment

emilio Jul 7, 2017

Choose a reason for hiding this comment

dylanmckay Jul 7, 2017

Choose a reason for hiding this comment

dylanmckay commented Jul 8, 2017

emilio commented Jul 8, 2017

bors-servo commented Jul 8, 2017

bors-servo commented Jul 8, 2017

bors-servo commented Jul 8, 2017

jethrogb commented Jul 8, 2017

emilio commented Jul 8, 2017

emilio commented Jul 8, 2017

dylanmckay commented Jul 8, 2017

emilio commented Jul 9, 2017

dylanmckay commented Jul 9, 2017