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

Suppressed lint warnings on dead-code and missing-docs for generated files. #16

Closed
wants to merge 1 commit into from

Conversation

Fraser999
Copy link

This just adds dead_code and missing_docs to the existing allowed lint checks. This will be a problem for any project trying to use forbid for these, but I don't see a good way round that.

@dwrensha
Copy link
Member

Thanks for the pull request!

Unfortunately, these changes do not cover all cases, and I worry that fixing it in only some cases might be confusing. In particular, if a schema has constants defined at the outermost level, they will still emit dead code warnings. The natural solution is to put the #![allow(...)] directives at the very top of the generated code, but then the generated code would not be importable with the include!() macro (cf. rust-lang/rfcs#752). That, in turn, would not be a problem if we could use the mod_path syntax extension, but it doesn't work on the stable release channel. So for now the recommended solution is to manually suppress the warnings.

Hmm. I suppose a somewhat nicer workaround might be to define an mod_capnp!() macro that automatically includes generated code under OUT_DIR and adds the allow directives.

What do you think?

@dwrensha
Copy link
Member

The proposed mod_capnp!() would expand

mod_capnp!(addressbook_capnp, "/addressbook_capnp.rs");

into

#[allow(dead_code, missing_docs)]
mod addressbook_capnp {
  include!(concat!(env!("OUT_DIR"), "/addressbook_capnp.rs"));
}

There could be a similar pub_mod_capnp!().

@Fraser999
Copy link
Author

Yeah - that looks like a better solution indeed - I hadn't considered outer scope constants.

I'm still new to Rust and Cap'n Proto, so forgive the question, but could we get away with only passing the path into the macro since the module name will always be the same as the filename (minus the extension) won't it?

Regardless, I'll close this pull request and await the macro fix :)

@Fraser999 Fraser999 closed this May 21, 2015
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.

2 participants