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

Use of ioctl!() in a crate with [warn(missing_docs)] #571

Closed
agrover opened this issue Apr 7, 2017 · 3 comments · Fixed by #661
Closed

Use of ioctl!() in a crate with [warn(missing_docs)] #571

agrover opened this issue Apr 7, 2017 · 3 comments · Fixed by #661
Labels

Comments

@agrover
Copy link
Contributor

agrover commented Apr 7, 2017

We'd like to enable [warn(missing_docs)] for our crate that uses nix, but when doing so we get this warning:

   Compiling devicemapper v0.4.3 (file:///home/agrover/git/stratis/devicemapper-rs)
warning: missing documentation for a function
  --> src/util.rs:11:1
   |
11 | ioctl!(read blkgetsize64 with 0x12, 114; u64);
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
note: lint level defined here
  --> src/lib.rs:43:9
   |
43 | #![warn(missing_docs)]
   |         ^^^^^^^^^^^^
   = note: this error originates in a macro outside of the current crate

    Finished dev [unoptimized + debuginfo] target(s) in 1.58 secs

I don't see a way in my nix-using crate to silence this (without disabling the warning, of course!) Thanks.

@kamalmarhubi
Copy link
Member

@agrover thanks for the report. I'm not too familiar with the ioctl stuff, so I can't answer immediately. In the meantime, does putting #[allow(missing_docs)] on that declaration work?

@Susurrus Susurrus added the A-bug label Jun 4, 2017
@roblabla
Copy link
Contributor

roblabla commented Jul 4, 2017

The ioctl! macro generates a pub function, so missing_docs will require you to document it. I haven't tried, but I'm pretty sure adding some docs above it would fix the issue, e.g. :

/// This is a binding to the blkgetsize64 ioctl
ioctl!(read blkgetsize64 with 0x12, 114; u64);

If blkgetsize64 shouldn't be publicly exported, then you should put it in a private module instead.

@roblabla
Copy link
Contributor

roblabla commented Jul 8, 2017

After some investigation, it turns out it's impossible to document functions generated by a macro in rust. See https://stackoverflow.com/questions/41361897/documenting-a-function-created-with-a-macro-in-rust.

So I'm wondering if it'd be possible to transform ioctl! to support something like

ioctl! {
    /// Some doc-comments
    readwrite some_ioctl(type: b'b', nr: 1, arg: some_arg)
}

This syntax looks dreadful so if someone has a better idea, feel free to comment. But the basic idea is to allow the doc comments to be inside the macro invocation so we can use it to generate the function with it.

bors bot added a commit that referenced this issue Jul 23, 2017
661: Allow doc attributes in ioctl r=asomers

fixes #571 . Note that this is a breaking change because it also changes 

```
ioctl!(some_name with 12);
```

to

```
ioctl!(bad some_name with 12);
```

This is to work around a bug in the rust compiler whereby rules around matching idents are overly strict. See rust-lang/rust#24189

It doesn't break anything else though.
@bors bors bot closed this as completed in #661 Jul 23, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants