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

Stabilize count, ignore, index, and length in Rust 1.80 #122808

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

c410-f3r
Copy link
Contributor

@c410-f3r c410-f3r commented Mar 21, 2024

Stabilization proposal

This PR proposes the stabilization of a subset of #![feature(macro_metavar_expr)] or more specifically, the stabilization of count, ignore, index and length.

Tracking issue: #83527
Version: 1.80 (June 13 2024 => beta, July 25 2024 => stable).

What is stabilized

Count

The number of times a meta variable repeats in total.

The output of count depends on where it is placed as well the provided index. If no index is provided, then it will always start at the innermost level.

macro_rules! count_idents {
    ( $( $i:ident ),* ) => {
        ${count($i)}
    };
}

fn main() {
    assert_eq!(count_idents!(a, b, c), 3);
}

Ignore

Binds a meta variable for repetition, but expands to nothing.

macro_rules! count {
    ( $( $i:stmt ),* ) => {{
        0 $( + 1 ${ignore($i)} )*
    }};
}

fn main() {
    assert_eq!(count!(if true {} else {}, let _: () = (), || false), 3);
}

Index

The current index of the inner-most repetition.

index is a counter that starts from 0 until the end of the repetition.

trait Foo {
    fn bar(&self) -> usize;
}

macro_rules! impl_tuple {
    ( $( $name:ident ),* ) => {
        impl<$( $name, )*> Foo for ($( $name, )*)
        where
            $( $name: AsRef<[u8]>, )*
        {
            fn bar(&self) -> usize {
                let mut sum: usize = 0;
                $({
                    const $name: () = ();
                    sum = sum.wrapping_add(self.${index()}.as_ref().len());
                })*
                sum
            }
        }
    };
}

impl_tuple!(A, B, C, D);

fn main() {
}

Length

The current index starting from the inner-most repetition.

length indicates the sum of meta variable elements, aka length, of the current repetition.

macro_rules! array_3d {
    ( $( $( $number:literal ),* );* ) => {
        [
            $(
                [
                    $( $number + ${length()}, )*
                ],
            )*
        ]
    };
}

fn main() {
    assert_eq!(array_3d!(0, 1; 2, 3; 4, 5), [[2, 3], [4, 5], [6, 7]]);
}

Motivation

Meta variable expressions not only facilitate the use of macros but also allow things that can't be done today like in the $index example.

An initial effort to stabilize this feature was made in #111908 but ultimately reverted because of possible obstacles related to syntax and expansion.

Nevertheless, #83527 (comment) tried to address some questions and fortunately the lang team accept #117050 the unblocking suggestions.

Here we are today after ~4 months so everything should be mature enough for wider use.

What isn't stabilized

$$ is not being stabilized due to unresolved concerns.

History

Tests

This list is a subset of https://github.com/rust-lang/rust/tree/master/tests/ui/macros/rfc-3086-metavar-expr.

Possible future work

With enough consensus, other nightly expressions can be added for experimentation and possibly stabilized in the future. For example, #118958.

Thanks @markbt for creating the RFC and thanks to @petrochenkov and @mark-i-m for reviewing the implementations.

@rustbot
Copy link
Collaborator

rustbot commented Mar 21, 2024

r? @fmease

rustbot has assigned @fmease.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Mar 21, 2024
@rust-log-analyzer

This comment has been minimized.

@c410-f3r c410-f3r changed the title Stabilize count, index, ignore and length in Rust 1.80 Stabilize count, ignore, index, and length in Rust 1.80 Mar 21, 2024
@fmease
Copy link
Member

fmease commented Mar 21, 2024

@c410-f3r Has this been documented in the Reference already? The box is not ticked over at the tracking issue. If not, stabilization is blocked on adding sufficient documentation.

@fmease fmease added needs-fcp This change is insta-stable, so needs a completed FCP to proceed. I-lang-nominated The issue / PR has been nominated for discussion during a lang team meeting. labels Mar 21, 2024
@joshtriplett

This comment was marked as outdated.

@rfcbot

This comment was marked as outdated.

@rfcbot rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Mar 21, 2024
@joshtriplett joshtriplett added T-lang Relevant to the language team, which will review and decide on the PR/issue. and removed T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Mar 21, 2024
@joshtriplett

This comment was marked as outdated.

@rfcbot

This comment was marked as outdated.

@rfcbot rfcbot removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Mar 21, 2024
@joshtriplett
Copy link
Member

@rfcbot merge

@rfcbot concern document-in-reference

@rfcbot
Copy link

rfcbot commented Mar 21, 2024

Team member @joshtriplett has proposed to merge this. The next step is review by the rest of the tagged team members:

Concerns:

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns.
See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added the proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. label Mar 21, 2024
@fmease fmease added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). S-waiting-on-fcp Status: PR is in FCP and is awaiting for FCP to complete. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. S-waiting-on-fcp Status: PR is in FCP and is awaiting for FCP to complete. labels Mar 21, 2024
@RalfJung
Copy link
Member

Length

The current index starting from the inner-most repetition.

What does this mean, and why does "length" not show up in the explanation of the length "function"? I would expect this to be the length of the inner-most repetition, is that not the case?

All these "functions" expand to integers -- of which type? Or do they end up being like integer literals without a suffix, i.e., the concrete integer type gets inferred following the usual rules?

@Veykril
Copy link
Member

Veykril commented Mar 21, 2024

Note that the tracking issue still has an unresolved question/bug open #83527

Figure out problems around hygiene

Has this been resolved?

@joshtriplett
Copy link
Member

@rfcbot concern hygiene-handled?

@Veykril
Copy link
Member

Veykril commented Mar 21, 2024

Ah actually, it seems the checkbox was never ticked? #95188

@joshtriplett
Copy link
Member

@rfcbot resolved hygiene-handled?

@lcnr
Copy link
Contributor

lcnr commented Mar 21, 2024

what is the difference between index and length?

@tgross35
Copy link
Contributor

Are the functions that take depth such as ${count(ident, depth)} included as part of this?

Nonblocker, diagnostics are pretty bare bones with no error codes or help

error: `count` can not be placed inside the inner-most repetition
 --> src/main.rs:7:19
  |
7 |             dbg!(${count($i, 0)});
  |                   ^^^^^^^^^^^^^^

@slanterns
Copy link
Contributor

Date seems incorrect :)

@nagisa
Copy link
Member

nagisa commented Mar 21, 2024

To follow up on the question on the depth of count, does "in total" mean this considers all levels of repetition? Like, if I write$( $a: ident: $($b: literal),* );+ and this matches a a: 1, 2, 3; b: 4, 5, 6, do I get count($b) of 3 twice, or a count($b) of 6 twice? Or does it depend on where I write the count at?

The RFC text does not really make this clear…

@c410-f3r
Copy link
Contributor Author

c410-f3r commented Mar 21, 2024

@fmease

Has this been documented in the Reference already?

No but documentation will be provided as soon as possible.

@RalfJung

What does this mean, and why does "length" not show up in the explanation of the length "function"? I would expect this to be the length of the inner-most repetition, is that not the case?

length indicates the sum of meta variable elements, aka length, of the current repetition and that is why, contrary the count, it can only be placed inside repetitions.

The nested repetitions of https://user-images.githubusercontent.com/17877264/158820060-035b2548-9ea9-4cbb-850b-e5836cf892fe.png illustrate better this concept.

More information is available at https://rust-lang.github.io/rfcs/3086-macro-metavar-expr.html#index-and-length

All these "functions" expand to integers -- of which type? Or do they end up being like integer literals without a suffix, i.e., the concrete integer type gets inferred following the usual rules?

length and count are expanded to unsuffixed integer literals.

More information is available at https://rust-lang.github.io/rfcs/3086-macro-metavar-expr.html#count

MetaVarExpr::Length(depth) => match repeats.iter().nth_back(depth) {
Some((_, length)) => {
result.push(TokenTree::token_alone(
TokenKind::lit(token::Integer, sym::integer(*length), None),
visited_span(),
));
}

@lcnr

what is the difference between index and length?

length indicates the sum of meta variable elements of the current repetition while index is a counter that starts from 0 until the end of the repetition.

macro_rules! index {
    ( $( $tt:tt ),* ) => {
        [$( ${ignore($tt)} ${index()}, )*]
    };
}

macro_rules! length {
    ( $( $tt:tt ),* ) => {
        [$( ${ignore($tt)} ${length()}, )*]
    };
}

fn main() {
    // 3 tokens generate an increment list of 3 elements starting from 0
    assert_eq!(index!(A, B, C), [0, 1, 2]);
    
    // 4 tokens generate a list of 4 elements containing 4
    assert_eq!(length!(A, B, C, D), [4, 4, 4, 4]);
}

More information is available at https://rust-lang.github.io/rfcs/3086-macro-metavar-expr.html#index-and-length

@ slanterns

Date seems incorrect :)

Thanks

@nagisa

To follow up on the question on the depth of count, does "in total" mean this considers all levels of repetition? Like, if I write $( $a: ident: $($b: literal),* );+ and this matches a a: 1, 2, 3; b: 4, 5, 6, do I get count($b) of 3 twice, or a count($b) of 6 twice? Or does it depend on where I write the count at?

count depends on where it is placed as well the provided index. If no index is provided, then it will always start at the innermost level.

So in your example a declaration of $( $a:ident: $($b: literal),* );+ called with a: 1, 2, 3; b: 4, 5, 6, will produce 3, 3 if called inside the outermost repetition or 6 if called without any repetition.

// Other examples

macro_rules! no_repetition {
    ( $( $a:ident: $( $b:literal ),* );+ ) => {
        [${count($b)}]
    };
}

macro_rules! no_repetition_with_index {
    ( $( $a:ident: $( $b:literal ),* );+ ) => {
        [${count($b, 1)}]
    };
}

macro_rules! outermost_repetition {
    ( $( $a:ident: $( $b:literal ),* );+ ) => {
        [$( ${ignore($a)} ${count($b)}, )+]
    };
}

fn main() {
    // 1 2 3 4 5 = 5 elements
    assert_eq!(no_repetition!(a: 1, 2, 3; b: 4, 5), [5]);
    
    // a b = 2 elements
    assert_eq!(no_repetition_with_index!(a: 1, 2, 3; b: 4, 5), [2]);
    
    // 1 2 3 = 3 elements, 4 5 = 2 elements
    assert_eq!(outermost_repetition!(a: 1, 2, 3; b: 4, 5), [3, 2]);
}

https://user-images.githubusercontent.com/17877264/158820060-035b2548-9ea9-4cbb-850b-e5836cf892fe.png might help you but it was created in a time when count started at the outermost level.

* PR updated

@jdahlstrom
Copy link

So does ${count()} equal ${length()} in the most common case of having only a single level of repetition?

@danielhenrymantilla
Copy link
Contributor

danielhenrymantilla commented Mar 21, 2024

@jdahlstrom
IIUC, I believe:

[$( $x / ${length()} ),*]

to be morally equivalent to:

let n = ${count(x)};
[$( $x / n ),*]
  • (notice where the operator lies w.r.t. the $()* repetition)

@tgross35
Copy link
Contributor

Would it be possible to prepare the PR for the reference before this lands? It seems like there is quite a bit of uncertainty about how all the features work, which makes me thing that perhaps this feature isn't very widely tested. As semantics have changed a few times since the RFC, that isn't the best reference either.

Quick search for usage that isn't in rustc forks https://github.com/search?q=lang%3Arust+%2F%28%3F-i%29%5C%24%5C%7B%28ignore%7Ccount%7Clength%7Cindex%29%5C%28.*%5C%7D%2F+-path%3Acompiler%2F+-path%3Alibrary%2F+-path%3Asrc%2Ftools%2Frust-analyzer&type=code

@c410-f3r
Copy link
Contributor Author

Has this been documented in the Reference already?

No but documentation will be provided as soon as possible.

As commented above, reference will be provided as soon as possible.

Semantic changed only once when count moved from outermost to innermost. #117050 also required the addition of $ but this is more syntax-related than an actual change of how the feature works, everything else remains the same as when the RFC was approved 2 years ago.

@workingjubilee
Copy link
Contributor

workingjubilee commented Mar 22, 2024

Is there any realistic pattern to suffix the integer literal expansions or nah?

@programmerjake
Copy link
Member

Is there any realistic pattern to suffix the integer literal expansions or nah?

well, I expect ${concat(${length()}, _u32)} to work at some point...

@bors
Copy link
Contributor

bors commented Apr 3, 2024

☔ The latest upstream changes (presumably #123390) made this pull request unmergeable. Please resolve the merge conflicts.

@nikomatsakis
Copy link
Contributor

nikomatsakis commented Apr 10, 2024

@rfcbot reviewed -- this would definitely be handy!

@jdahlstrom
Copy link

jdahlstrom commented Apr 10, 2024

I brought this up in the doc PR but it belongs here – length should probably be renamed len before stabilization. The latter is de facto standard in the standard library, whereas the former is only used in a single unstable API. These metafunctions aren’t library items of course, but should presumably still be consistent with established names.

@workingjubilee
Copy link
Contributor

I'm slightly worried about random inference regressions punishing this particularly hard if there's no standard/easy way to proof the emitted literals against such. I guess we can use a block expression for each one?

@bors
Copy link
Contributor

bors commented Apr 24, 2024

☔ The latest upstream changes (presumably #104087) made this pull request unmergeable. Please resolve the merge conflicts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet