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

cargo fix: Panic with "Cannot replace slice of data that was already replaced" #13030

Open
camelid opened this issue Mar 12, 2022 · 6 comments
Open
Labels
C-bug Category: bug Command-fix S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review

Comments

@camelid
Copy link
Member

camelid commented Mar 12, 2022

I have a PR to add a new suggestion to rustc: rust-lang/rust#88672. It uses Diagnostic::multipart_suggestions since there are multiple ways for the user to change their code. These multiple suggestions are exclusive since they modify the same code.

However, rustfix is panicking when I use run-rustfix in a test: "Cannot replace slice of data that was already replaced".

The relevant code in my PR is here: https://github.com/rust-lang/rust/blob/6a89e97ab14cd4c6be70b47fd139c87fd90150b7/compiler/rustc_parse/src/parser/diagnostics.rs#L226-L234
and here: https://github.com/rust-lang/rust/blob/6a89e97ab14cd4c6be70b47fd139c87fd90150b7/compiler/rustc_parse/src/parser/diagnostics.rs#L1272-L1277

I'm not sure how to fix this problem in rustfix, but I'm willing to try with some mentoring :)

@camelid
Copy link
Member Author

camelid commented Mar 12, 2022

@ehuss
Copy link
Contributor

ehuss commented Mar 14, 2022

Diagnostics with multiple exclusive options shouldn't be marked as MachineApplicable (as rustfix obviously can't handle them).

To handle this, rustfix would need to gain the ability to provide a user-interactive multiple-choice UI for choosing which fix to apply. There is some discussion of something like this in #13028. It is feasible, though I personally don't think it is very valuable. I prefer to interact within my editor and apply suggestions by clicking quick-fix buttons. That means you don't have to go out and use a separate tool, and makes it easier to see the changes in context, and is generally faster (for me).

@camelid
Copy link
Member Author

camelid commented Mar 23, 2022

Hi @ehuss, thanks for your quick reply! I would have responded sooner but I missed your message :)

The multiple exclusive options actually are not marked as MachineApplicable. They were marked as MaybeIncorrect and then later Unspecified, but I'm still getting the panic. Any idea why this is happening?

@ehuss
Copy link
Contributor

ehuss commented Mar 23, 2022

If you mean by a UI test, then you'll want to remove the run-rustfix header. That header will apply all suggestions ignoring the applicability. There's also rustfix-only-machine-applicable, but I don't think that applies to your case (assuming no suggestions are machine-applicable).

@camelid
Copy link
Member Author

camelid commented Mar 24, 2022

Ah, I didn't realize that it applied suggestions indiscriminately! I moved the exclusive suggestions to another test since otherwise the generated .fixed file was invalid. Thanks so much for your help!

Is this panic still considered a bug (and thus I should leave the issue open) or is it considered a sign of something wrong with rustc's output that is thus not a rustfix bug?

@ehuss
Copy link
Contributor

ehuss commented Mar 24, 2022

I'd leave it open. Even though these types of suggestions aren't currently supported, it probably shouldn't panic.

@ehuss ehuss changed the title Panic with "Cannot replace slice of data that was already replaced" cargo fix: Panic with "Cannot replace slice of data that was already replaced" Nov 22, 2023
@ehuss ehuss transferred this issue from rust-lang/rustfix Nov 22, 2023
@ehuss ehuss added C-bug Category: bug S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review Command-fix labels Nov 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: bug Command-fix S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review
Projects
None yet
Development

No branches or pull requests

2 participants