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

Comments are copied verbatim without checking if they might be invalid #426

Closed
jethrogb opened this issue Jan 24, 2017 · 11 comments
Closed
Labels

Comments

@jethrogb
Copy link
Contributor

C and Rust comments are not the same. bindgen copies comments verbatim anyway, and this breaks things.

For example, here's an excerpt from a real-life header I was using with bindgen 0.19:

#include <stddef.h>
#include <stdint.h>

typedef uint32_t mbedtls_mpi_uint;

/**
 * \brief          MPI structure
 */
typedef struct
{
    int s;              /*!<  integer sign      */
    size_t n;           /*!<  total # of limbs  */
    mbedtls_mpi_uint *p;          /*!<  pointer to limbs  */
}
mbedtls_mpi;

bindgen 0.20 output for this type is:

/* automatically generated by rust-bindgen */

pub type mbedtls_mpi_uint = u32;
/**
 * \brief          MPI structure
 */
#[repr(C)]
#[derive(Debug, Copy)]
pub struct _bindgen_ty_2 {
    /*!<  integer sign      */
    pub s: ::std::os::raw::c_int,
    /*!<  total # of limbs  */
    pub n: usize,
    /*!<  pointer to limbs  */
    pub p: *mut mbedtls_mpi_uint,
}
#[test]
fn bindgen_test_layout__bindgen_ty_2() {
    assert_eq!(::std::mem::size_of::<_bindgen_ty_2>() , 24usize);
    assert_eq!(::std::mem::align_of::<_bindgen_ty_2>() , 8usize);
}
impl Clone for _bindgen_ty_2 {
    fn clone(&self) -> Self { *self }
}
pub use self::_bindgen_ty_2 as mbedtls_mpi;

which rustc does not like:

error: expected outer doc comment
  --> <anon>:10:5
   |
10 |     /*!<  integer sign      */
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
   = note: inner doc comments like this (starting with `//!` or `/*!`) can only appear before items
@emilio
Copy link
Contributor

emilio commented Jan 24, 2017

Oh, that sucks, is it really a parse error?

cc rust-lang/rust#38825 and #378.

@emilio
Copy link
Contributor

emilio commented Jan 24, 2017

It seems we need to process comments no matter what, sigh.

@emilio emilio added the bug label Jan 24, 2017
@jethrogb
Copy link
Contributor Author

I think it shouldn't be that hard. If the comment is C-style and the first character is * or !, insert a space. Otherwise, if the first character is / or !, insert a space.

@ghost
Copy link

ghost commented Jan 26, 2017

I'm having this issue as well. This seems like it should be very common, since there are a lot of libraries with Doxygen comments. Is there at least a way to turn off the copying of comments? What if I don't need annotations? At the very least, what @jethrogb suggested would at least let me use bindgen. Everything else about the bindings is great!

EDIT: the specific header I'm having issues with: https://github.com/opensource-apple/CF/blob/master/CFArray.h#L28

@ghost
Copy link

ghost commented Jan 26, 2017

It looks like we need to fix this here, based on the following discussion of these kinds of errors:
rust-lang/rust#2789

emilio added a commit to emilio/rust-bindgen that referenced this issue Jan 26, 2017
This is mostly a work around rust-lang#426,
until we implement the proper fix.
@emilio
Copy link
Contributor

emilio commented Jan 26, 2017

I'm landing a workaround in #444, since I don't have the time to write the proper fix right now. This should allow people to keep using bindgen meanwhile.

emilio added a commit to emilio/rust-bindgen that referenced this issue Jan 26, 2017
This is mostly a work around rust-lang#426,
until we implement the proper fix.
bors-servo pushed a commit that referenced this issue Jan 26, 2017
codegen: Add an option to skip comment generation.

This is mostly a work around #426,
until we implement the proper fix.

r? @fitzgen
@mattmacy
Copy link

Same issue when building FAST
mmacy@DuhBuntu [~/devel/ruFAST|18:11|146] cargo build
Compiling ruFAST v0.1.0 (file:///storage/mmacy/devel/ruFAST)
error: expected outer doc comment
--> /storage/mmacy/devel/ruFAST/target/debug/build/ruFAST-d825d3ad93645a05/out/bindings.rs:41738:1
|
41738 | /*! \brief Class interface for cl_mem.
| ^ starting here...
41739 | | *
41740 | | * \note Copies of these objects are shallow, meaning that the copy will refer
41741 | | * to the same underlying cl_mem as the original. For details, see
41742 | | * clRetainMemObject() and clReleaseMemObject().
41743 | | *
41744 | | * \see cl_mem
41745 | | */
| |
__^ ...ending here
|
= note: inner doc comments like this (starting with //! or /*!) can only appear before items

error: Could not compile ruFAST.

To learn more, run the command again with --verbose.

@shnewto
Copy link
Contributor

shnewto commented Feb 21, 2017

@mattmacy assuming since you're running cargo build you've got a build.rs. did you set the "skip comment generation" flag there?
bindgen::builder().generate_comments(false)

@mattmacy
Copy link

@Snewt yes, I got past that point by doing so

@emilio
Copy link
Contributor

emilio commented Apr 22, 2017

@ildar suggests in #650 converting them to rustdoc. It's slightly tricky, but should be doable.

@rpjohnst
Copy link

rpjohnst commented May 1, 2017

A related issue is that sometimes comments aren't actually doc comments, like with this sort of pattern:

#if !SOME_DEFINE
    int field_a;
    int field_b;
#endif //!SOME_DEFINE

dylanmckay added a commit to dylanmckay/rust-bindgen that referenced this issue Jul 6, 2017
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 added a commit to dylanmckay/rust-bindgen that referenced this issue Jul 6, 2017
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 added a commit to dylanmckay/rust-bindgen that referenced this issue Jul 6, 2017
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 added a commit to dylanmckay/rust-bindgen that referenced this issue Jul 6, 2017
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 added a commit to dylanmckay/rust-bindgen that referenced this issue Jul 6, 2017
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 added a commit to dylanmckay/rust-bindgen that referenced this issue Jul 6, 2017
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 added a commit to dylanmckay/rust-bindgen that referenced this issue Jul 6, 2017
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 added a commit to dylanmckay/rust-bindgen that referenced this issue Jul 6, 2017
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 added a commit to dylanmckay/rust-bindgen that referenced this issue Jul 7, 2017
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.
bors-servo pushed a commit that referenced this issue Jul 8, 2017
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.
joseluis added a commit to dankamongmen/notcurses that referenced this issue Aug 9, 2020
Fixes bindgen errors due to invalid Rust comments imported from functions `ncmultiselector_options` and `ncselector_item`.

Related:
- rust-lang/rust-bindgen#426
- rust-lang/rust-bindgen#1749
joseluis added a commit to dankamongmen/libnotcurses-sys that referenced this issue Sep 16, 2021
Fixes bindgen errors due to invalid Rust comments imported from functions `ncmultiselector_options` and `ncselector_item`.

Related:
- rust-lang/rust-bindgen#426
- rust-lang/rust-bindgen#1749
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants