[Derive] `#[doc = macro_name!()]` doesn't play well with doc comments

epage commented 2 years ago

Issue by rami3l Thursday Jul 29, 2021 at 19:22 GMT Originally opened as https://github.com/clap-rs/clap/issues/2639

Please complete the following tasks

[X] I have searched the discussions
[X] I have searched the existing issues

Rust Version

1.54.0

Clap Version

master

Minimal reproducible code

#[test]
fn doc_comments_raw_macro() {
    macro_rules! doc_lorem_ipsum {
        () => {
            "Lorem ipsum"
        };
        (inner) => {
            "Fooify a bar and a baz"
        };
    }

    #[doc = doc_lorem_ipsum!()]
    #[derive(Clap, PartialEq, Debug)]
    struct LoremIpsum {
        #[doc = doc_lorem_ipsum!(inner)]
        #[clap(short, long)]
        foo: bool,
    }

    let help = get_long_help::<LoremIpsum>();
    assert!(help.contains("Lorem ipsum"));
    assert!(help.contains("Fooify a bar and a baz"));
}

Steps to reproduce the bug with the above code

Run the test above.

Actual Behaviour

Assertions fail with no comments help message detected:

%%% LONG_HELP %%%:=====
clap_derive 

USAGE:
    clap_derive [FLAGS]

FLAGS:
    -f, --foo

    -h, --help
            Prints help information

    -V, --version
            Prints version information

=====

Expected Behaviour

Assertions pass with all help messages specified by the macro call.

%%% LONG_HELP %%%:=====
clap_derive 
<- We should see some help messages here...

USAGE:
    clap_derive [FLAGS]

FLAGS:
    -f, --foo      <- And here...

    -h, --help
            Prints help information

    -V, --version
            Prints version information

=====

Additional Context

No response

Debug Output

No response

epage commented 2 years ago

Comment by rami3l Thursday Jul 29, 2021 at 20:47 GMT

I investigated the current codebase for quite a while, and the main problem we have here seems to be the following:

When will the macro be expanded? Ideally clap_derive should never bother to deal with the macro call itself, but should instead send it safely to call site. Is this feasible?

epage commented 2 years ago

Comment by epage Thursday Jul 29, 2021 at 21:12 GMT

I went down a different rabbit hole, in hopes that bumping MSRV and maybe proc macro deps might be of assistance. I'm guessing not but at least we have https://github.com/clap-rs/clap/pull/2640

I'll look into that part. Part of the problem is we do post-processing on the doc comment to better format it. iirc we have a verbatim flag which could work with it. I'll dig a little into these parts.

epage commented 2 years ago

Comment by epage Thursday Jul 29, 2021 at 21:17 GMT

From https://github.com/rust-lang/rust/pull/83366

Expansion of macro expressions in "inert" attributes occurs after decorators have executed, analogously to macro expressions appearing in the function body or other parts of decorator input.

There is currently no way for decorators to accept macros in key-value position if macro expansion must be performed before the decorator executes (if the macro can simply be copied into the output for later expansion, that can work).

Still fairly weak on my proc macros but I think this is confirming that we have to pass along the macro call for later expansion, which means it is unlikely to work with clap_derive without some re-work. As-is, we always do post-processing, even with verbatim is enabled.

epage commented 2 years ago

Comment by epage Thursday Jul 29, 2021 at 21:18 GMT

I guess one option is we can either generate or embed some const-fns that do the post-processing for us.

epage commented 2 years ago

Comment by rami3l Thursday Jul 29, 2021 at 21:49 GMT

@epage Yeah, I saw that post-processing part.

Before, with only doc literals, it's reasonable to do this in the derive macro. However, when the doc macros come into play, it seems impossible (at least to both of us) to keep it this way: the call site has to be changed.

But don't worry, once the macro call has been transferred to the call side, we should be able to use something like the tt_call hack to perform comptime preprocessing. (I used it in my own app pacaptr and it works, although it's rather hard to get right.)

It's true that const fn might be another possibility. It's however also quite complicated to operate on &str these days with only const fn... :(

Of course, we have to be very careful about clap's dependencies... But in the worst case, we can at least move the preprocessing part to runtime.

epage commented 2 years ago

Comment by rami3l Thursday Jul 29, 2021 at 23:57 GMT

@epage Wait a second, does https://github.com/rust-lang/rust/pull/87264 include the eager expansion that we want? If so, we can wait for it!

(And in that case I know exactly which function should be modified!)

epage / clapng