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

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

Open
2 tasks done
rami3l opened this issue Jul 29, 2021 · 9 comments
Open
2 tasks done

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

rami3l opened this issue Jul 29, 2021 · 9 comments
Labels
A-derive Area: #[derive]` macro API C-bug Category: Updating dependencies E-hard Call for participation: Experience needed to fix: Hard / a lot

Comments

@rami3l
Copy link
Contributor

rami3l commented Jul 29, 2021

Please complete the following tasks

  • I have searched the discussions
  • 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

@rami3l rami3l added the C-bug Category: Updating dependencies label Jul 29, 2021
@rami3l
Copy link
Contributor Author

rami3l commented Jul 29, 2021

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
Copy link
Member

epage commented Jul 29, 2021

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 #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
Copy link
Member

epage commented Jul 29, 2021

From rust-lang/rust#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
Copy link
Member

epage commented Jul 29, 2021

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

@rami3l
Copy link
Contributor Author

rami3l commented Jul 29, 2021

@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.

@rami3l
Copy link
Contributor Author

rami3l commented Jul 29, 2021

@epage Wait a second, does rust-lang/rust#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!)

@pksunkara pksunkara added A-derive Area: #[derive]` macro API E-hard Call for participation: Experience needed to fix: Hard / a lot labels Jul 30, 2021
@brunoczim
Copy link

This is a big problem to me. I need to generate commands from a macro, and the macro documents commands automatically, but it needs to interpolate a few strings inside documentation.

I am using #[doc = concat!("foo ", $bar, " baz")] to achive this, but it breaks clap and so I cannot both document and generate help/about at once.

@rami3l
Copy link
Contributor Author

rami3l commented Jan 19, 2022

@brunoczim Since rust-lang/rust#87264 has already landed with expand_expr, I think I can take a look.

... But don't expect that to be available in clap soon... We have to wait for rust-lang/rust#90765 and there's still an MSRV to be bumped :(

@brunoczim
Copy link

I'd appreciate it, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-derive Area: #[derive]` macro API C-bug Category: Updating dependencies E-hard Call for participation: Experience needed to fix: Hard / a lot
Projects
None yet
Development

No branches or pull requests

4 participants