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

[RFC-3086] Consider out-of-bound depths of count #111923

Closed
wants to merge 1 commit into from

Conversation

c410-f3r
Copy link
Contributor

@c410-f3r c410-f3r commented May 24, 2023

Fix #111905

In the matching of macro calls and their respective declarations (transcribe), a warning is issued if count has a depth greater than the number of nested NamedMatchs through be verification of MatchedSeq with empty elements.

Doesn't affect ( $( { $( [ $( ( $( $t:ident )* ) )* ] )* } )* ) => { ${count(t, 1)} } called with bar!( { [] [] } ) which will still continue to output 2.

@rustbot
Copy link
Collaborator

rustbot commented May 24, 2023

r? @davidtwco

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels May 24, 2023
@c410-f3r
Copy link
Contributor Author

c410-f3r commented Jun 6, 2023

r? compiler

@rustbot rustbot assigned oli-obk and unassigned davidtwco Jun 6, 2023
@oli-obk
Copy link
Contributor

oli-obk commented Jun 7, 2023

I'm on vacation and won't get to this before the 19th

Also cc @petrochenkov for macro stuff

@oli-obk
Copy link
Contributor

oli-obk commented Jun 15, 2023

@bors try

@oli-obk oli-obk added T-lang Relevant to the language team, which will review and decide on the PR/issue. needs-fcp This change is insta-stable, so needs a completed FCP to proceed. and removed T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 15, 2023
@bors
Copy link
Contributor

bors commented Jun 15, 2023

⌛ Trying commit c218557ac737a43e768bd2736ead296b3fb6a7b5 with merge 5723d0448b9ce6bed8424358575f87427ecc6050...

@bors

This comment was marked as outdated.

@oli-obk

This comment was marked as outdated.

@craterbot

This comment was marked as outdated.

@craterbot craterbot added S-waiting-on-crater Status: Waiting on a crater run to be completed. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 15, 2023
@craterbot

This comment was marked as outdated.

@c410-f3r
Copy link
Contributor Author

@oli-obk count is part of the nightly macro_metavar_expr feature and this PR is fixing a logical error that should be rare to appear in practice.

needs-fcp: This change is insta-stable, so needs a completed FCP to proceed.

That said, I think a FCP isn't necessary.

@oli-obk oli-obk removed the needs-fcp This change is insta-stable, so needs a completed FCP to proceed. label Jun 18, 2023
@oli-obk
Copy link
Contributor

oli-obk commented Jun 18, 2023

Oh I didn't realize that. I just saw the behaviour change...

@craterbot cancel

@craterbot

This comment was marked as outdated.

@oli-obk
Copy link
Contributor

oli-obk commented Jun 18, 2023

@craterbot abort

@craterbot

This comment was marked as resolved.

@craterbot craterbot removed the S-waiting-on-crater Status: Waiting on a crater run to be completed. label Jun 18, 2023
@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Aug 29, 2023
@bors
Copy link
Contributor

bors commented Aug 29, 2023

⌛ Testing commit 0366888 with merge ce62cbcd4aba08c32632d7805d200eaf53a96a98...

@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Contributor

bors commented Aug 29, 2023

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 29, 2023
@c410-f3r
Copy link
Contributor Author

Well, the matter needs more investigation on my side

@oli-obk
Copy link
Contributor

oli-obk commented Aug 29, 2023

@rustbot author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 29, 2023
@ehuss
Copy link
Contributor

ehuss commented Sep 3, 2023

@bors r-

bors synchronize queue fix

@Dylan-DPC
Copy link
Member

@c410-f3r any updates on this?

@c410-f3r
Copy link
Contributor Author

CI failure still can't be reproduced locally.

@oli-obk Can you r? again so I can see the logs? All previous runs are now 404.

@rustbot
Copy link
Collaborator

rustbot commented Oct 30, 2023

Failed to set assignee to again: cannot assign: response: {"message":"Not Found","documentation_url":"https://docs.github.com/rest/issues/assignees#add-assignees-to-an-issue"}

Note: Only org members with at least the repository "read" role, users with write permissions, or people who have commented on the PR may be assigned.

@oli-obk
Copy link
Contributor

oli-obk commented Oct 30, 2023

@bors r+

@bors
Copy link
Contributor

bors commented Oct 30, 2023

📌 Commit 82ba5fb has been approved by oli-obk

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 30, 2023
@bors
Copy link
Contributor

bors commented Oct 30, 2023

⌛ Testing commit 82ba5fb with merge 33e81da...

bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 30, 2023
[RFC-3086] Consider out-of-bound depths of `count`

Fix rust-lang#111905

In the matching of macro calls and their respective declarations (transcribe), a warning is issued if `count` has a depth greater than the number of nested `NamedMatch`s through be verification of `MatchedSeq` with empty elements.

Doesn't affect `( $( { $( [ $( ( $( $t:ident )* ) )* ] )* } )* ) => { ${count(t, 1)} }` called with `bar!( { [] [] } )` which will still continue to output `2`.
@rust-log-analyzer
Copy link
Collaborator

The job i686-gnu failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
-   --> $DIR/issue-111905.rs:4:26
+ error: only unsuffixes integer literals are supported in meta-variable expressions
+   --> $DIR/issue-111905.rs:4:27
3    |
4 LL |     ($($t:ident)*) => { ${count(t, 4294967296)} };
+    |                           ^^^^^
6 
- error: related fragment that refers the `count` meta-variable expression was not found
-   --> $DIR/issue-111905.rs:9:60
-   --> $DIR/issue-111905.rs:9:60
+ error: only unsuffixes integer literals are supported in meta-variable expressions
+   --> $DIR/issue-111905.rs:9:61
9    |
10 LL |     ( $( { $( [ $( ( $( $t:ident )* ) )* ] )* } )* ) => { ${count(t, 4294967296)} }
+    |                                                             ^^^^^
12 
- error: aborting due to 2 previous errors
+ error: expected expression, found `$`
+ error: expected expression, found `$`
+   --> $DIR/issue-111905.rs:4:25
+    |
+ LL |     ($($t:ident)*) => { ${count(t, 4294967296)} };
+ ...
+ LL |     foo!();
+    |     ------ in this macro invocation
+    |
+    |
+    = note: this error originates in the macro `foo` (in Nightly builds, run with -Z macro-backtrace for more info)
+ 
+ error: expected expression, found `$`
+   --> $DIR/issue-111905.rs:9:59
+    |
+ LL |     ( $( { $( [ $( ( $( $t:ident )* ) )* ] )* } )* ) => { ${count(t, 4294967296)} }
+ ...
+ ...
+ LL |     bar!( { [] [] } );
+    |
+    = note: this error originates in the macro `bar` (in Nightly builds, run with -Z macro-backtrace for more info)
+ 
+ error: aborting due to 4 previous errors
---
To only update this specific test, also pass `--test-args macros/rfc-3086-metavar-expr/issue-111905.rs`

error: 1 errors occurred comparing output.
status: exit status: 1
command: RUSTC_ICE="0" "/checkout/obj/build/i686-unknown-linux-gnu/stage2/bin/rustc" "/checkout/tests/ui/macros/rfc-3086-metavar-expr/issue-111905.rs" "-Zthreads=1" "-Zsimulate-remapped-rust-src-base=/rustc/FAKE_PREFIX" "-Ztranslate-remapped-path-to-local-path=no" "-Z" "ignore-directory-in-diagnostics-source-blocks=/cargo" "--sysroot" "/checkout/obj/build/i686-unknown-linux-gnu/stage2" "--target=i686-unknown-linux-gnu" "--error-format" "json" "--json" "future-incompat" "-Ccodegen-units=1" "-Zui-testing" "-Zdeduplicate-diagnostics=no" "-Zwrite-long-types-to-disk=no" "-Cstrip=debuginfo" "--emit" "metadata" "-C" "prefer-dynamic" "--out-dir" "/checkout/obj/build/i686-unknown-linux-gnu/test/ui/macros/rfc-3086-metavar-expr/issue-111905" "-A" "unused" "-A" "internal_features" "-Crpath" "-Cdebuginfo=0" "-Lnative=/checkout/obj/build/i686-unknown-linux-gnu/native/rust-test-helpers" "-L" "/checkout/obj/build/i686-unknown-linux-gnu/test/ui/macros/rfc-3086-metavar-expr/issue-111905/auxiliary"
--- stderr -------------------------------
error: only unsuffixes integer literals are supported in meta-variable expressions
##[error]  --> /checkout/tests/ui/macros/rfc-3086-metavar-expr/issue-111905.rs:4:27
   |
   |
LL |     ($($t:ident)*) => { ${count(t, 4294967296)} };

error: only unsuffixes integer literals are supported in meta-variable expressions
##[error]  --> /checkout/tests/ui/macros/rfc-3086-metavar-expr/issue-111905.rs:9:61
   |
   |
LL |     ( $( { $( [ $( ( $( $t:ident )* ) )* ] )* } )* ) => { ${count(t, 4294967296)} }

error: expected expression, found `$`
##[error]  --> /checkout/tests/ui/macros/rfc-3086-metavar-expr/issue-111905.rs:4:25
   |
   |
LL |     ($($t:ident)*) => { ${count(t, 4294967296)} };
...
LL |     foo!();
   |     ------ in this macro invocation
   |
   |
   = note: this error originates in the macro `foo` (in Nightly builds, run with -Z macro-backtrace for more info)

error: expected expression, found `$`
##[error]  --> /checkout/tests/ui/macros/rfc-3086-metavar-expr/issue-111905.rs:9:59
   |
LL |     ( $( { $( [ $( ( $( $t:ident )* ) )* ] )* } )* ) => { ${count(t, 4294967296)} }
...
...
LL |     bar!( { [] [] } );
   |
   = note: this error originates in the macro `bar` (in Nightly builds, run with -Z macro-backtrace for more info)

error: aborting due to 4 previous errors

@bors
Copy link
Contributor

bors commented Oct 30, 2023

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Oct 30, 2023
@c410-f3r
Copy link
Contributor Author

c410-f3r commented Oct 31, 2023

Thank you @oli-obk

I spent several hours trying to find a solution and nothing worked. Now I don't have the time or motivation to continue pursuing this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. 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.

${count()} disregards depth parameter when captured repetition is empty