Allow for missing invisible close delim when reparsing an expression. by nnethercote · Pull Request #139298 · rust-lang/rust

nnethercote · 2025-04-03T03:45:03Z

This can happen when invalid syntax is passed to a declarative macro. We shouldn't be too strict about the token stream position once the parser has rejected the invalid syntax.

Fixes #139248.

r? @petrochenkov

try-job: test-various

petrochenkov · 2025-04-03T14:37:21Z

@bors r+ rollup

bors · 2025-04-03T14:37:25Z

📌 Commit a2a081a has been approved by petrochenkov

It is now in the queue for this repository.

nnethercote · 2025-04-03T22:23:13Z

#137874 is similar to #139248, there might be a common fix slightly different to what I've done here.

@bors r-

nnethercote · 2025-04-03T22:41:22Z

#137874 is similar to #139248, there might be a common fix slightly different to what I've done here.

No, it needs a separate fix.

@bors r=petrochenkov

bors · 2025-04-03T22:41:25Z

📌 Commit a2a081a has been approved by petrochenkov

It is now in the queue for this repository.

…nkov Allow for missing invisible close delim when reparsing an expression. This can happen when invalid syntax is passed to a declarative macro. We shouldn't be too strict about the token stream position once the parser has rejected the invalid syntax. Fixes rust-lang#139248. r? `@petrochenkov`

Rollup of 14 pull requests Successful merges: - rust-lang#137869 (Demote i686-pc-windows-gnu to Tier 2) - rust-lang#137880 (Autodiff batching) - rust-lang#138546 (Add integer to string formatting tests) - rust-lang#138947 (Refactor Apple version handling in the compiler) - rust-lang#138950 (replace extra_filename with strict version hash in metrics file names) - rust-lang#139213 (Run coretests and alloctests with cg_clif in CI) - rust-lang#139274 (Rustdoc: typecheck settings.js) - rust-lang#139295 (Remove creation of duplicate `AnonPipe`) - rust-lang#139298 (Allow for missing invisible close delim when reparsing an expression.) - rust-lang#139313 (Deduplicate some `rustc_middle` function bodies by calling the `rustc_type_ir` equivalent) - rust-lang#139317 (compiletest: Encapsulate all of the code that touches libtest) - rust-lang#139322 (Add helper function for checking LLD usage to `run-make-support`) - rust-lang#139335 (Pass correct param-env to `error_implies`) - rust-lang#139342 (Add a mailmap entry for myself) Failed merges: - rust-lang#138949 (Rename `is_like_osx` to `is_like_darwin`) r? `@ghost` `@rustbot` modify labels: rollup

Zalathar · 2025-04-04T05:45:17Z

Possibly failed in rollup: #139344 (comment)

@bors r-

Zalathar · 2025-04-04T05:46:33Z

@bors try

bors · 2025-04-04T05:47:44Z

⌛ Trying commit a2a081a with merge 46cc200...

Allow for missing invisible close delim when reparsing an expression. This can happen when invalid syntax is passed to a declarative macro. We shouldn't be too strict about the token stream position once the parser has rejected the invalid syntax. Fixes rust-lang#139248. r? `@petrochenkov` --- try-job: test-various

bors · 2025-04-04T06:41:39Z

💔 Test failed - checks-actions

nnethercote · 2025-04-04T06:55:04Z

I don't understand what's going wrong. The test passes locally and the diff on CI is strange:

41    |
42 LL | thread_local! { static a : () = (if b) }
43    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected expression
-   --> $SRC_DIR/std/src/sys/thread_local/native/mod.rs:LL:COL
45    |
46    = note: while parsing argument for this `expr` macro fragment
47    |

Zalathar · 2025-04-04T07:08:43Z

Oh, I wonder if this is wasm causing problems because the path of the source file defining the macro is different.

In the raw stderr, the path on line 44 is:

    --> /rustc/FAKE_PREFIX/library/std/src/sys/thread_local/statik.rs:28:18

nnethercote · 2025-04-04T08:40:10Z

How is wasm involved?

I could rewrite the test to use a local macro instead of thread_local!, that might help.

nnethercote · 2025-04-06T21:59:44Z

@bors try

Allow for missing invisible close delim when reparsing an expression. This can happen when invalid syntax is passed to a declarative macro. We shouldn't be too strict about the token stream position once the parser has rejected the invalid syntax. Fixes rust-lang#139248. r? `@petrochenkov` --- try-job: test-various

bors · 2025-04-06T22:00:56Z

⌛ Trying commit ad9d8f9 with merge 281c69c...

nnethercote · 2025-04-06T22:48:28Z

@bors try

bors · 2025-04-06T22:49:39Z

⌛ Trying commit 296b484 with merge 9840dcf...

Allow for missing invisible close delim when reparsing an expression. This can happen when invalid syntax is passed to a declarative macro. We shouldn't be too strict about the token stream position once the parser has rejected the invalid syntax. Fixes rust-lang#139248. r? `@petrochenkov` --- try-job: test-various

nnethercote · 2025-04-07T00:15:23Z

@bors try

bors · 2025-04-07T00:16:34Z

⌛ Trying commit fbdf9f8 with merge 732ba88...

Allow for missing invisible close delim when reparsing an expression. This can happen when invalid syntax is passed to a declarative macro. We shouldn't be too strict about the token stream position once the parser has rejected the invalid syntax. Fixes rust-lang#139248. r? `@petrochenkov` --- try-job: test-various

bors · 2025-04-07T01:41:41Z

☀️ Try build successful - checks-actions
Build commit: 732ba88 (732ba88caf76736237d422d1580a6865c071b6ff)

nnethercote · 2025-04-07T01:55:41Z

I could rewrite the test to use a local macro instead of thread_local!, that might help.

That worked. I still don't understand what went wrong with the thread_local! version.

@bors r=petrochenkov

bors · 2025-04-07T01:55:43Z

📌 Commit fbdf9f8 has been approved by petrochenkov

It is now in the queue for this repository.

Zalathar · 2025-04-07T02:20:24Z

That worked. I still don't understand what went wrong with the thread_local! version.

I believe the problem was:

When using thread_local!, the error message includes a path into standard library source where thread_local_inner is defined ($SRC_DIR/std/src/sys/thread_local/native/mod.rs:LL:COL).
Some targets use a different definition of thread_local_inner that is in a different file, so the path into standard library source is different.
That different path causes the snapshots to not match, even though the difference isn't important to what this test is testing.

All of that is a distraction from what this test actually cares about, so I think switching to a local macro was the right call.

…nkov Allow for missing invisible close delim when reparsing an expression. This can happen when invalid syntax is passed to a declarative macro. We shouldn't be too strict about the token stream position once the parser has rejected the invalid syntax. Fixes rust-lang#139248. r? `@petrochenkov` --- try-job: test-various

nnethercote · 2025-04-07T02:25:56Z

so I think switching to a local macro was the right call.

Thanks for the explanation!

Rollup of 4 pull requests Successful merges: - rust-lang#138314 (fix usage of `autodiff` macro with inner functions) - rust-lang#138766 (coverage: Deal with unused functions and their names in one place) - rust-lang#139298 (Allow for missing invisible close delim when reparsing an expression.) - rust-lang#139426 (Make the UnifyKey and UnifyValue imports non-nightly) r? `@ghost` `@rustbot` modify labels: rollup

This can happen when invalid syntax is passed to a declarative macro. We shouldn't be too strict about the token stream position once the parser has rejected the invalid syntax. Fixes rust-lang#139248.

petrochenkov · 2025-04-07T16:21:28Z

This is included into #139464 so I'll close.

rustbot assigned petrochenkov Apr 3, 2025

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 Apr 3, 2025

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-review Status: Awaiting review from the assignee but also interested parties. labels Apr 3, 2025

bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 3, 2025

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 Apr 3, 2025

Zalathar mentioned this pull request Apr 4, 2025

Rollup of 14 pull requests #139344

Closed

bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 4, 2025

This comment has been minimized.

Sign in to view

nnethercote force-pushed the fix-139248 branch from a2a081a to ad9d8f9 Compare April 6, 2025 21:59

This comment has been minimized.

Sign in to view

nnethercote force-pushed the fix-139248 branch from ad9d8f9 to 296b484 Compare April 6, 2025 22:48

This comment has been minimized.

Sign in to view

nnethercote force-pushed the fix-139248 branch from 296b484 to fbdf9f8 Compare April 6, 2025 23:03

This comment has been minimized.

Sign in to view

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 Apr 7, 2025

Zalathar mentioned this pull request Apr 7, 2025

Rollup of 4 pull requests #139460

Closed

Allow for missing invisible close delim when reparsing an expression.

eefb755

This can happen when invalid syntax is passed to a declarative macro. We shouldn't be too strict about the token stream position once the parser has rejected the invalid syntax. Fixes rust-lang#139248.

nnethercote force-pushed the fix-139248 branch from fbdf9f8 to eefb755 Compare April 7, 2025 03:54

petrochenkov closed this Apr 7, 2025

Uh oh!

Conversation

nnethercote commented Apr 3, 2025 • edited by Zalathar Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

petrochenkov commented Apr 3, 2025

Uh oh!

bors commented Apr 3, 2025

Uh oh!

nnethercote commented Apr 3, 2025

Uh oh!

nnethercote commented Apr 3, 2025

Uh oh!

bors commented Apr 3, 2025

Uh oh!

Zalathar commented Apr 4, 2025

Uh oh!

Zalathar commented Apr 4, 2025

Uh oh!

bors commented Apr 4, 2025

Uh oh!

This comment has been minimized.

bors commented Apr 4, 2025

Uh oh!

nnethercote commented Apr 4, 2025

Uh oh!

Zalathar commented Apr 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nnethercote commented Apr 4, 2025

Uh oh!

nnethercote commented Apr 6, 2025

Uh oh!

bors commented Apr 6, 2025

Uh oh!

This comment has been minimized.

nnethercote commented Apr 6, 2025

Uh oh!

bors commented Apr 6, 2025

Uh oh!

This comment has been minimized.

This comment has been minimized.

nnethercote commented Apr 7, 2025

Uh oh!

bors commented Apr 7, 2025

Uh oh!

bors commented Apr 7, 2025

Uh oh!

nnethercote commented Apr 7, 2025

Uh oh!

bors commented Apr 7, 2025

Uh oh!

Zalathar commented Apr 7, 2025

Uh oh!

nnethercote commented Apr 7, 2025

Uh oh!

petrochenkov commented Apr 7, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

nnethercote commented Apr 3, 2025 •

edited by Zalathar

Loading

Zalathar commented Apr 4, 2025 •

edited

Loading