Skip to content

Revert "Avoid nested replacement ranges" from #129346. #132587

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Nov 5, 2024
Merged

Revert "Avoid nested replacement ranges" from #129346. #132587

merged 1 commit into from
Nov 5, 2024

Conversation

nnethercote
Copy link
Contributor

It caused a test regression in the cfg_eval.rs crate. (The bugfix in #129346 was in a different commit; this commit was just a code simplification.)

r? @petrochenkov

@rustbot

This comment was marked as outdated.

Sign in to view
@rustbot

This comment was marked as resolved.

Sign in to view
@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 Nov 4, 2024
@nnethercote
Revert "Avoid nested replacement ranges" from rust-lang#129346.
981dc02 
It caused a test regression in the `cfg_eval.rs` crate. (The bugfix
in rust-lang#129346 was in a different commit; this commit was just a code
simplification.)
@nnethercote nnethercote force-pushed the revert-avoid-nested-replacement-ranges branch from 7d1fe0d to 981dc02 Compare November 4, 2024 04:57
@nnethercote
Copy link
Contributor Author

The regression was reported by @danielhenrymantilla here.

@nnethercote nnethercote mentioned this pull request Nov 4, 2024
Merged
@nnethercote
Copy link
Contributor Author

I don't entirely understand the failing example, but reverting the change fixes it. I haven't added a test because the example involves a proc macro and is hard to write a test for :/

@danielhenrymantilla
Copy link
Contributor

Awesome thanks @nnethercote ❤️

  • I tried to minimize the error with no proc-macro involvement, but that would "fix" the issue; so it does seem related to having some proc-macro passing of tokens and whatnot. I can try to invest a bit more investigative work into reducing the issue, should the tests in this repo allow defining ad-hoc proc-macros.

@petrochenkov
Copy link
Contributor

@bors r+

@bors
Copy link
Collaborator

bors commented Nov 4, 2024

📌 Commit 981dc02 has been approved by petrochenkov

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-review Status: Awaiting review from the assignee but also interested parties. labels Nov 4, 2024
workingjubilee added a commit to workingjubilee/rustc that referenced this pull request Nov 4, 2024
@workingjubilee
Rollup merge of rust-lang#132587 - nnethercote:revert-avoid-nested-re…
3f9bbc8 
…placement-ranges, r=petrochenkov

Revert "Avoid nested replacement ranges" from rust-lang#129346.

It caused a test regression in the `cfg_eval.rs` crate. (The bugfix in rust-lang#129346 was in a different commit; this commit was just a code simplification.)

r? `@petrochenkov`
@workingjubilee workingjubilee mentioned this pull request Nov 4, 2024
Closed
@nnethercote
Copy link
Contributor Author

should the tests in this repo allow defining ad-hoc proc-macros

They do, see tests/ui/proc-macro and tests/ui/proc-macro/auxiliary.

bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 4, 2024
@bors
Auto merge of rust-lang#132616 - workingjubilee:rollup-0ykzbyb, r=wor…
404c53a 
…kingjubilee

Rollup of 10 pull requests

Successful merges:

 - rust-lang#129884 (mark some target features as 'forbidden' so they cannot be (un)set with -Ctarget-feature)
 - rust-lang#132153 (Stabilise `const_char_encode_utf16`.)
 - rust-lang#132473 ([core/fmt] Replace checked slice indexing by unchecked to support panic-free code)
 - rust-lang#132571 (add const_eval_select macro to reduce redundancy)
 - rust-lang#132587 (Revert "Avoid nested replacement ranges" from rust-lang#129346.)
 - rust-lang#132596 ([rustdoc] Fix `--show-coverage` when JSON output format is used)
 - rust-lang#132598 (Clippy: Move some attribute lints to be early pass (post expansion))
 - rust-lang#132601 (Update books)
 - rust-lang#132606 (Improve example of `impl Pattern for &[char]`)
 - rust-lang#132609 (docs: fix grammar in doc comment at unix/process.rs)

r? `@ghost`
`@rustbot` modify labels: rollup
@nnethercote nnethercote added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Nov 4, 2024
@nnethercote
Copy link
Contributor Author

@lqd said this:

the regression is indeed present in the 1.83 unexpected test failures: https://crater-reports.s3.amazonaws.com/beta-1.83-1/beta-2024-10-19/gh/danielhenrymantilla.cfg_eval.rs/log.txt

so I have added the beta-nominated label.

@workingjubilee workingjubilee mentioned this pull request Nov 5, 2024
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 5, 2024
@bors
Auto merge of rust-lang#132626 - workingjubilee:rollup-hbmtbzk, r=wor…
27e38f8 
…kingjubilee

Rollup of 11 pull requests

Successful merges:

 - rust-lang#131153 (Improve duplicate derive Copy/Clone diagnostics)
 - rust-lang#132025 (fix suggestion for diagnostic error E0027)
 - rust-lang#132303 (More tests for non-exhaustive C-like enums in FFI)
 - rust-lang#132492 (remove support for extern-block const intrinsics)
 - rust-lang#132587 (Revert "Avoid nested replacement ranges" from rust-lang#129346.)
 - rust-lang#132596 ([rustdoc] Fix `--show-coverage` when JSON output format is used)
 - rust-lang#132598 (Clippy: Move some attribute lints to be early pass (post expansion))
 - rust-lang#132601 (Update books)
 - rust-lang#132606 (Improve example of `impl Pattern for &[char]`)
 - rust-lang#132608 (document `type_implements_trait`)
 - rust-lang#132609 (docs: fix grammar in doc comment at unix/process.rs)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit f8ac0e7 into rust-lang:master Nov 5, 2024
6 checks passed
@rustbot rustbot added this to the 1.84.0 milestone Nov 5, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Nov 5, 2024
@rust-timer
Unrolled build for rust-lang#132587
7b08cc3 
Rollup merge of rust-lang#132587 - nnethercote:revert-avoid-nested-replacement-ranges, r=petrochenkov

Revert "Avoid nested replacement ranges" from rust-lang#129346.

It caused a test regression in the `cfg_eval.rs` crate. (The bugfix in rust-lang#129346 was in a different commit; this commit was just a code simplification.)

r? `@petrochenkov`
@nnethercote nnethercote deleted the revert-avoid-nested-replacement-ranges branch November 5, 2024 22:26
@jieyouxu jieyouxu mentioned this pull request Nov 7, 2024
Open
@apiraino
Copy link
Contributor

apiraino commented Nov 7, 2024

Beta backport accepted as per compiler team on Zulip. A backport PR will be authored by the release team at the end of the current development cycle.

@rustbot label +beta-accepted

@rustbot rustbot added the beta-accepted Accepted for backporting to the compiler in the beta channel. label Nov 7, 2024
@cuviper cuviper mentioned this pull request Nov 7, 2024
Merged
@cuviper cuviper modified the milestones: 1.84.0, 1.83.0 Nov 7, 2024
@cuviper cuviper removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Nov 7, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 8, 2024
@bors
Auto merge of rust-lang#132733 - cuviper:beta-next, r=cuviper
aa7626b 
[beta] backports

- rustdoc: skip stability inheritance for some item kinds rust-lang#132481
- Avoid use imports in thread_local_inner! in static rust-lang#132101
- Also treat `impl` definition parent as transparent regarding modules rust-lang#132453
- Revert "Avoid nested replacement ranges" from rust-lang#129346. rust-lang#132587

r? cuviper
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 8, 2024
@bors
Auto merge of rust-lang#132733 - cuviper:beta-next, r=cuviper
eb6e3fa 
[beta] backports

- rustdoc: skip stability inheritance for some item kinds rust-lang#132481
- Avoid use imports in thread_local_inner! in static rust-lang#132101
- Also treat `impl` definition parent as transparent regarding modules rust-lang#132453
- Revert "Avoid nested replacement ranges" from rust-lang#129346. rust-lang#132587

r? cuviper
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@petrochenkov petrochenkov

Labels
beta-accepted Accepted for backporting to the compiler in the beta channel. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.83.0
Development

Successfully merging this pull request may close these issues.
7 participants
@nnethercote @rustbot @danielhenrymantilla @petrochenkov @bors @apiraino @cuviper