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

Nested invocation in substitutions #28

Closed
Tracked by #31
Emoun opened this issue Jun 1, 2021 · 5 comments
Closed
Tracked by #31

Nested invocation in substitutions #28

Emoun opened this issue Jun 1, 2021 · 5 comments
Labels
D-accepted A decision (D) has been made and the issue will be worked on I-feature This issue (I) regards a (potential) feature in the project T-accepted Triage (T): Initial review accepted issue/PR as valid

Comments

@Emoun
Copy link
Owner

Emoun commented Jun 1, 2021

Short Description:

Enable nested invocations inside substitution blocks.

Motivation:

Take #26, where we have many struct members needing to be instantiated to the same value:

impl Example {
  fn inline_new() -> Self {
    Example { one: 0, two: 0, .. }
  }

  fn attr_new() -> Self {
    Example {one: 0, two: 0, ..}
  }
}

We would like to be able to make a single invocation that inserts the instantiations many places. The closest we get today is:

#[duplicate(
  members [one: 0, two: 0, ..]
)]
impl Example {
  fn inline_new() -> Self {
    Example {members}
  }

  fn attr_new() -> Self {
    Example {members}
  }
}

However, this repeats the :0 , bit for every member.

Nested invocation would be able to remove this repetition. However, it is not available inside substitutions. Enabling it, would allow us to do:

#[duplicate(
  members [ #duplicate[
      mem; [one]; [two]
    ][ 
      mem: 0, 
  ]]
)]
impl Example {
  fn inline_new() -> Self {
    Example {members}
  }

  fn attr_new() -> Self {
    Example {members}
  }
}

Design

The first problem with enabling nested invocations inside substitutions is how to handle user attributes. Since we currently use the syntax #[..][..] to denote nested invocation, this prohibits users from using attributes inside substitutions, as they would be assumed to be nested invocations.

To alleviate this, we change the syntax of nested invocations to #duplicate[..][..]. This is generally illegal Rust syntax, so no user would write this outside specifically when nested invocations are wanted. However, it is valid syntax, allowing us to parse it.

This is, if course, a breaking change, as it will also be the syntax use for existing nested invocations.

Misc:

Syntax alternatives

  1. #[duplicate( .. )]: This mimics top-level invocations. However, it means we cannot distinguish between nested invocations and new top-level invocations that happen to be in the substitutions.
@Emoun Emoun added D-discussion A decision (D) has not been made yet and is open to discussion I-feature This issue (I) regards a (potential) feature in the project labels Jun 1, 2021
@ghost ghost added the T-new Triage (T): Has yet to be reviewed label Jun 1, 2021
@Emoun Emoun added T-accepted Triage (T): Initial review accepted issue/PR as valid and removed T-new Triage (T): Has yet to be reviewed labels Jun 1, 2021
@Emoun Emoun mentioned this issue Sep 14, 2021
12 tasks
@Emoun
Copy link
Owner Author

Emoun commented Sep 14, 2021

There is little difference between a nested invocation and a new top-level invocation that happens to be in a substitution, except for when they are expanded and therefore the legality.

Thought experiment:

Given a nested invocation that produces X.
After the nested invocation is expanded, the parent invocation now produces Y+X.
Replacing the nested invocation with a new top level invocation, W (assuming doing so would be legal), the parent invocation now produces Y+W. W is then further expanded into X, meaning the result is Y+X.

Therefore, the only difference is whether a given invocation would result in macro-legal intermediate Rust expansions.
Since nested invocations are expanded by duplicate itself, rust never manages to see any intermediate expansion.
This means nested invocations are strictly more powerful than top-level invocations that happen to be in substitutions (as rust will have a look at the substituted code and therefore get a change to reject it).

It would therefore not be problematic to change the nested substitution syntax to #[duplicate(...)][] based on distinction. This does have other downsides however:

  • use ... as that changes duplicate's name to something else will not be honored for nested invocation as we won't know it and will be checking for the literal duplicate string.
  • When using duplicate_inline, nested invocations would still have to use only duplicate, which could be confusing. This could be fixed, but that would mean 2 different strings do the same thing (since nested imports already work mostly like duplicate_inline).

@Emoun
Copy link
Owner Author

Emoun commented Sep 21, 2021

Another alternative:

Anywhere in either the invocation or the body, any presence of #[duplicate] or duplicate_inline will immediately call those functions on the following token tree. This is conceptually simple while being very powerful.

First, it solves the problem presented in this issue, but also another one discussed in #26 regarding repetition in the body.
I.e. where you would like to perform repetition in the body without having to code it into the invocation.

We would still have the problem of this not playing well with use duplicate as something_else. However, I expect this to be a small issue and can be addressed in the documentation.

@Emoun
Copy link
Owner Author

Emoun commented Oct 7, 2021

Regarding the alternative in the previous comment:

Looking for #[duplicate] is a difficult process, as attributes are applied to items. What is an acceptable item is not trivial, and some items don't accept attributes. Implementing this probably requires using syn at least.
Implementing the same for duplicate_inline is much easier, as we can just take the following token (assuming its {}, [], or ().

@Emoun
Copy link
Owner Author

Emoun commented Dec 25, 2021

Final decision: Use duplicate_inline for nested invocation and nothing else.

@Emoun Emoun added D-accepted A decision (D) has been made and the issue will be worked on and removed D-discussion A decision (D) has not been made yet and is open to discussion labels Dec 25, 2021
@Emoun Emoun mentioned this issue Dec 25, 2021
@Emoun
Copy link
Owner Author

Emoun commented Feb 8, 2022

Nested invocations have been upgraded to work everywhere, which includes this use case. Awaiting next release.

@Emoun Emoun closed this as completed Feb 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
D-accepted A decision (D) has been made and the issue will be worked on I-feature This issue (I) regards a (potential) feature in the project T-accepted Triage (T): Initial review accepted issue/PR as valid
Projects
None yet
Development

No branches or pull requests

1 participant