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

Nicer syntax for include_cpp #42

Closed
adetaylor opened this issue Oct 16, 2020 · 8 comments · Fixed by #73
Closed

Nicer syntax for include_cpp #42

adetaylor opened this issue Oct 16, 2020 · 8 comments · Fixed by #73

Comments

@adetaylor
Copy link
Collaborator

The current syntax:

include_cpp!(Header("bob.h"), Allow("get_foo"), Allow("Bar"))

has four problems:

  • It's ugly.
  • It doesn't allow the name of the ffi mod to be specified.
  • It doesn't allow specification of whether the ffi mod is pub, etc.
  • There's no good way of attaching documentation to directives like Allow, etc.

I am thinking of switching to a syntax a bit more like cxx:

#[autocxx::bridge]
pub mod ffi {
   include!("bob.h") # or maybe include_cpp!
   allow!("get_foo")
   allow!("Bar")
}

Hopefully I can persuade rustdoc to treat all the inner macros as document-able things. If not, they may become functions (which do nothing when called as actual functions, but can have docs attached). The problem there is that a single crate can either export a macro or functions. We'll see.

I should do this at the same time as #37 to have one major breaking change, though I don't think anyone's relying on the existing syntax yet.

The new syntax isn't perfect, since I wanted the include_cpp! line to act as much as possible like #include in C++. So thinking still in progress.

@dtolnay
Copy link
Contributor

dtolnay commented Oct 16, 2020

I would recommend against an attribute macro.

  • Advantage of attribute macro: The only reason cxx::bridge really needs to be an attribute macro is so that rustfmt is able to comprehend the contents, since we need to have real Rust signatures in there. But for autocxx the contents of the macro input are going to have not much to do with Rust syntax.
  • Disadvantage of attribute macro: You get heinous error messages if the macro is not resolved, since rustc will just assume it isn't important and then continue compiling the body of the module as well as the rest of the crate, without having run the macro.

For the autocxx use case I would probably expect something resembling:

autocxx::bridge! {
    #include "bob.h"
    ...
}

@adetaylor
Copy link
Collaborator Author

OK, I'll aim for that. Thanks. I want to come up with something that looks nice after rustfmtting so that might slightly dictate the choice too.

@adetaylor
Copy link
Collaborator Author

@dtolnay...

You get heinous error messages if the macro is not resolved, since rustc will just assume it isn't important

Dare I say, that feels like a bug? Or at least a questionable design decision especially given the Rust preference for failing fast?

I'm still quite attached to the idea of an attribute macro, not really for the rustfmt reason, but because it enables me to attach documentation to directives within the macro not just the outermost macro.

I've tried fiddling around with function-style macros but I can't find a way to achieve that desirable property. To understand the nuances of (say) AllowPOD a reader would need to look it up within the long documentation for include_cxx!.

@dtolnay
Copy link
Contributor

dtolnay commented Oct 19, 2020

Do you mean (looking at the snippet at the top of the thread) documentation of allow! and similar macros? If so, wouldn't that work exactly the same whether or not the outer macro invocation is an attribute? Either way you'd still expose allow! as a macro that if ever invoked by rustc would print a compile_error showing the intended use of include_cxx.

@dtolnay
Copy link
Contributor

dtolnay commented Oct 19, 2020

Dare I say, that feels like a bug?

Yeah -- sadly the implementation of procedural macros is pretty awful and this isn't among the biggest things wrong with it. :(

@adetaylor
Copy link
Collaborator Author

Do you mean (looking at the snippet at the top of the thread) documentation of allow! and similar macros?

Yes.

If so, wouldn't that work exactly the same whether or not the outer macro invocation is an attribute?

I missed an important detail - I'm thinking of contexts like IDEs (VSCode, specifically) where the help text pops up when you hover over something, and you can do things like "go to definition" etc. (I'm assuming the emission of help is driven by rust-analyzer and therefore this applies to all IDEs not just VSCode, but then again maybe I'm overthinking this and nobody really uses IDE contextual help.)

As it happens, help text does not seem to pop up very reliably for function-like macros anyway at the moment, but I think that's probably rust-analyzer churn, it's worked in the past. It sometimes works :)

I'm not sure this is an important enough reason to suffer the enumerated perils of attribute macros, but I do like the idea of people being able to get direct and immediate help on the different instructions they can give to autocxx.

@dtolnay
Copy link
Contributor

dtolnay commented Oct 19, 2020

Ah I see, good call.

I found it's possible to make it work as follows, but this gets pretty deep into workaround territory so isn't necessarily advisable. But it supports both hover documentation and click-through on both include and inner with rust-analyzer.

use testing::outer;

outer! {
    #include "test.h"

    inner!(...)
    inner!(...)
}
// lib.rs

/// (DOCUMENTATION OF OUTER...)
#[macro_export]
macro_rules! outer {
    (
        $(#$include:ident $lit:literal)*
        $($mac:ident!($($arg:tt)*))*
    ) => {
        $($crate::$include!{__docs})*
        $($crate::$mac!{__docs})*
        $crate::do_the_work! {
            $(#include $lit)*
            $($mac!($($arg)*))*
        }
    };
}

/// (DOCUMENTATION OF INCLUDE...)
#[macro_export]
macro_rules! include {
    ($($tt:tt)*) => { $crate::usage!{$($tt)*} };
}

/// (DOCUMENTATION OF INNER...)
#[macro_export]
macro_rules! inner {
    ($($tt:tt)*) => { $crate::usage!{$($tt)*} };
}

#[doc(hidden)]
#[macro_export]
macro_rules! usage {
    (__docs) => {};
    ($($tt:tt)*) => {
        compile_error! {r#"usage:  outer! {
                   #include "path/to/header.h"
                   inner!(...)
               }
"#}
    };
}

#[doc(hidden)]
#[macro_export]
macro_rules! do_the_work { // FIXME: this is a procedural macro
    ($($tt:tt)*) => {};
}

@adetaylor
Copy link
Collaborator Author

Thanks. That looks like witchcraft. I like it.

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 a pull request may close this issue.

2 participants