cxx icon indicating copy to clipboard operation
cxx copied to clipboard

Use /// when propagating doc comments

Open alexeyr opened this issue 3 years ago • 1 comments

In #616 I've noticed // is used for the comments; using /// instead will allow Doxygen to recognize them as doc comments.

alexeyr avatar Jan 12 '21 11:01 alexeyr

It seems not the case. We have some docstrings in the rust part that are indeed as /// but they are "copied" in the generate files as // instead of the format understood by Doxygen doc

It can be useful to have them "copied" with the right format so that Doxygen can be used to generate docs for the bindings.

gabrik avatar Feb 10 '22 12:02 gabrik

@dtolnay regarding https://github.com/dtolnay/cxx/pull/1048#discussion_r1020063842

I understand that it could be simple for someone to work around, but I am not interested in accepting this PR as currently implemented. Taking Rust docs and turning them into either doxygen docs or silently into something that looks like doxygen docs, but isn't, is going to come across as clearly broken behavior to anybody that uses this.

Doxygen has quite a few rules to comply with (e.g. "To document a member of a C++ class, you must also document the class itself."), and having the full validation to ensure a compliant conversion in CXX may be an overkill and passing this responsibility to its users instead is IMHO ok. But I understand that "feature" shouldn't be called "doxygen" in order not to set wrong expectations. Perhaps what could work is to have an optional cxx_doc attribute which could be used to control how comments look like in the generated files, e.g.:

#[cxx::bridge(namespace = "org::blobstore")]
mod ffi {
    /// Shared structs with fields visible to both languages.
    #[cxx_doc(comment_line = "//!")
    struct BlobMetadata {
...

or one could perhaps also use that to have a different comment content:

#[cxx::bridge(namespace = "org::blobstore")]
mod ffi {
    /// Shared structs with fields visible to both languages.
    #[cxx_doc(comment_line = "///", text = " Shared structs with fields\n visible to both languages.")
    struct BlobMetadata {
...

@alexeyr @gabrik what do you think?

tomtau avatar Nov 17 '22 03:11 tomtau

I don't find that an appealing approach.

Regarding the "to document a member of a C++ class, you must also document the class itself" rule, I agree we don't need cxx to check that.

dtolnay avatar Nov 17 '22 03:11 dtolnay

@dtolnay what is your preference? Keep it outside CXX and leave it to custom post-processing scripts to be applied on generated files?

tomtau avatar Nov 17 '22 04:11 tomtau