Add `Steal::is_stolen()` #128815

Nadrieril · 2024-08-08T08:24:12Z

Writers of rustc drivers (such as myself) often encounter stealing issues. It is currently impossible to gracefully handle them. This PR adds a Steal::is_stolen() function for that purpose.

rustbot · 2024-08-08T08:24:20Z

r? @davidtwco

rustbot has assigned @davidtwco.
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

jieyouxu · 2024-08-08T09:07:46Z

Looks reasonable to me. I think this is fine to add since rustc_data_structures is perma-unstable anyways right? Feel free to r=me after PR CI is green.

r? jieyouxu

lqd · 2024-08-08T09:36:24Z

Sometimes people remove unused code even if it's pub, adding a comment explaining why it exists would help ensure that it's not deleted. The PR's description would work fine for this comment.

cjgillot · 2024-08-08T10:31:18Z

Note that this method must not be used from inside a rustc query, as that would carry untracked information across queries. Could you add a comment and eventually tweak one of the internal lints?

compiler/rustc_data_structures/src/steal.rs

Co-authored-by: lcnr <rust@lcnr.de>

lcnr · 2024-08-09T08:58:05Z

@bors r=jieyouxu,lcnr

bors · 2024-08-09T08:58:08Z

📌 Commit c966370 has been approved by jieyouxu,lcnr

It is now in the queue for this repository.

lcnr · 2024-08-09T08:58:12Z

@bors rollup=always

Add `Steal::is_stolen()` Writers of rustc drivers (such as myself) often encounter stealing issues. It is currently impossible to gracefully handle them. This PR adds a `Steal::is_stolen()` function for that purpose.

…iaskrgr Rollup of 6 pull requests Successful merges: - rust-lang#128742 (miri: make vtable addresses not globally unique) - rust-lang#128815 (Add `Steal::is_stolen()`) - rust-lang#128859 (Fix the name of signal 19 in library/std/src/sys/pal/unix/process/process_unix/tests.rs for mips/sparc linux) - rust-lang#128864 (Use `SourceMap::end_point` instead of `- BytePos(1)` in arg removal suggestion) - rust-lang#128865 (Ensure let stmt compound assignment removal suggestion respect codepoint boundaries) - rust-lang#128874 (Disable verbose bootstrap command failure logging by default) r? `@ghost` `@rustbot` modify labels: rollup

Add `Steal::is_stolen()` Writers of rustc drivers (such as myself) often encounter stealing issues. It is currently impossible to gracefully handle them. This PR adds a `Steal::is_stolen()` function for that purpose.

bjorn3 · 2024-08-09T15:11:22Z

compiler/rustc_data_structures/src/steal.rs

+    ///
+    /// This should not be used within rustc as it leaks information not tracked
+    /// by the query system, breaking incremental compilation.
+    pub fn is_stolen(&self) -> bool {


Maybe call it untracked_is_stolen()? We tend to prefix methods that don't properly go through the incr comp system with untracked_.

Ah, I didn't know that. I was in the process of adding an internal lint to warn about uses of functions like this as was suggested ^^. Do you think the lint would be worth it?

There are probably other methods which don't have the untracked_ prefix. Either renaming those or adding a lint for them would be beneficial I think.

Add `Steal::is_stolen()` Writers of rustc drivers (such as myself) often encounter stealing issues. It is currently impossible to gracefully handle them. This PR adds a `Steal::is_stolen()` function for that purpose.

…iaskrgr Rollup of 9 pull requests Successful merges: - rust-lang#128815 (Add `Steal::is_stolen()`) - rust-lang#128817 (VxWorks code refactored ) - rust-lang#128822 (add `builder-config` into tarball sources) - rust-lang#128838 (rustdoc: do not run doctests with invalid langstrings) - rust-lang#128852 (use stable sort to sort multipart diagnostics) - rust-lang#128859 (Fix the name of signal 19 in library/std/src/sys/pal/unix/process/process_unix/tests.rs for mips/sparc linux) - rust-lang#128864 (Use `SourceMap::end_point` instead of `- BytePos(1)` in arg removal suggestion) - rust-lang#128865 (Ensure let stmt compound assignment removal suggestion respect codepoint boundaries) - rust-lang#128874 (Disable verbose bootstrap command failure logging by default) r? `@ghost` `@rustbot` modify labels: rollup

…llaumeGomez Rollup of 8 pull requests Successful merges: - rust-lang#128815 (Add `Steal::is_stolen()`) - rust-lang#128822 (add `builder-config` into tarball sources) - rust-lang#128838 (rustdoc: do not run doctests with invalid langstrings) - rust-lang#128852 (use stable sort to sort multipart diagnostics) - rust-lang#128859 (Fix the name of signal 19 in library/std/src/sys/pal/unix/process/process_unix/tests.rs for mips/sparc linux) - rust-lang#128864 (Use `SourceMap::end_point` instead of `- BytePos(1)` in arg removal suggestion) - rust-lang#128865 (Ensure let stmt compound assignment removal suggestion respect codepoint boundaries) - rust-lang#128874 (Disable verbose bootstrap command failure logging by default) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#128815 - Nadrieril:is_stolen, r=jieyouxu,lcnr Add `Steal::is_stolen()` Writers of rustc drivers (such as myself) often encounter stealing issues. It is currently impossible to gracefully handle them. This PR adds a `Steal::is_stolen()` function for that purpose.

…llot Add an internal lint that warns when accessing untracked data Some methods access data that is not tracked by the query system and should be used with caution. As suggested in rust-lang#128815 (comment), in this PR I propose a lint (modeled on the `potential_query_instability` lint) that warns when using some specially-annotatted functions. I can't tell myself if this lint would be that useful, compared to renaming `Steal::is_stolen` to `is_stolen_untracked`. This would depend on whether there are other functions we'd want to lint like this. So far it seems they're called `*_untracked`, which may be clear enough. r? `@oli-obk`

…llot Add an internal lint that warns when accessing untracked data Some methods access data that is not tracked by the query system and should be used with caution. As suggested in rust-lang#128815 (comment), in this PR I propose a lint (modeled on the `potential_query_instability` lint) that warns when using some specially-annotatted functions. I can't tell myself if this lint would be that useful, compared to renaming `Steal::is_stolen` to `is_stolen_untracked`. This would depend on whether there are other functions we'd want to lint like this. So far it seems they're called `*_untracked`, which may be clear enough. r? ``@oli-obk``

…llot Add an internal lint that warns when accessing untracked data Some methods access data that is not tracked by the query system and should be used with caution. As suggested in rust-lang#128815 (comment), in this PR I propose a lint (modeled on the `potential_query_instability` lint) that warns when using some specially-annotatted functions. I can't tell myself if this lint would be that useful, compared to renaming `Steal::is_stolen` to `is_stolen_untracked`. This would depend on whether there are other functions we'd want to lint like this. So far it seems they're called `*_untracked`, which may be clear enough. r? ```@oli-obk```

…llot Add an internal lint that warns when accessing untracked data Some methods access data that is not tracked by the query system and should be used with caution. As suggested in rust-lang#128815 (comment), in this PR I propose a lint (modeled on the `potential_query_instability` lint) that warns when using some specially-annotatted functions. I can't tell myself if this lint would be that useful, compared to renaming `Steal::is_stolen` to `is_stolen_untracked`. This would depend on whether there are other functions we'd want to lint like this. So far it seems they're called `*_untracked`, which may be clear enough. r? `@oli-obk`

…llot Add an internal lint that warns when accessing untracked data Some methods access data that is not tracked by the query system and should be used with caution. As suggested in rust-lang#128815 (comment), in this PR I propose a lint (modeled on the `potential_query_instability` lint) that warns when using some specially-annotatted functions. I can't tell myself if this lint would be that useful, compared to renaming `Steal::is_stolen` to `is_stolen_untracked`. This would depend on whether there are other functions we'd want to lint like this. So far it seems they're called `*_untracked`, which may be clear enough. r? ``@oli-obk``

…llot Add an internal lint that warns when accessing untracked data Some methods access data that is not tracked by the query system and should be used with caution. As suggested in rust-lang#128815 (comment), in this PR I propose a lint (modeled on the `potential_query_instability` lint) that warns when using some specially-annotatted functions. I can't tell myself if this lint would be that useful, compared to renaming `Steal::is_stolen` to `is_stolen_untracked`. This would depend on whether there are other functions we'd want to lint like this. So far it seems they're called `*_untracked`, which may be clear enough. r? ```@oli-obk```

…llot Add an internal lint that warns when accessing untracked data Some methods access data that is not tracked by the query system and should be used with caution. As suggested in rust-lang#128815 (comment), in this PR I propose a lint (modeled on the `potential_query_instability` lint) that warns when using some specially-annotatted functions. I can't tell myself if this lint would be that useful, compared to renaming `Steal::is_stolen` to `is_stolen_untracked`. This would depend on whether there are other functions we'd want to lint like this. So far it seems they're called `*_untracked`, which may be clear enough. r? ````@oli-obk````

Rollup merge of rust-lang#128919 - Nadrieril:lint-query-leaks, r=cjgillot Add an internal lint that warns when accessing untracked data Some methods access data that is not tracked by the query system and should be used with caution. As suggested in rust-lang#128815 (comment), in this PR I propose a lint (modeled on the `potential_query_instability` lint) that warns when using some specially-annotatted functions. I can't tell myself if this lint would be that useful, compared to renaming `Steal::is_stolen` to `is_stolen_untracked`. This would depend on whether there are other functions we'd want to lint like this. So far it seems they're called `*_untracked`, which may be clear enough. r? ``@oli-obk``

Add an internal lint that warns when accessing untracked data Some methods access data that is not tracked by the query system and should be used with caution. As suggested in rust-lang/rust#128815 (comment), in this PR I propose a lint (modeled on the `potential_query_instability` lint) that warns when using some specially-annotatted functions. I can't tell myself if this lint would be that useful, compared to renaming `Steal::is_stolen` to `is_stolen_untracked`. This would depend on whether there are other functions we'd want to lint like this. So far it seems they're called `*_untracked`, which may be clear enough. r? ``@oli-obk``

rustbot assigned davidtwco Aug 8, 2024

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 Aug 8, 2024

Nadrieril mentioned this pull request Aug 8, 2024

Fix attempted to read from stolen value hacspec/hax#27

Open

rustbot assigned jieyouxu and unassigned davidtwco Aug 8, 2024

jieyouxu approved these changes Aug 8, 2024

View reviewed changes

Add Steal::is_stolen()

09ae438

Nadrieril force-pushed the is_stolen branch from 0a35ec7 to 09ae438 Compare August 8, 2024 10:11

lcnr reviewed Aug 8, 2024

View reviewed changes

compiler/rustc_data_structures/src/steal.rs Outdated Show resolved Hide resolved

Tweak wording

c966370

Co-authored-by: lcnr <rust@lcnr.de>

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 Aug 9, 2024

matthiaskrgr mentioned this pull request Aug 9, 2024

Rollup of 6 pull requests #128877

Closed

matthiaskrgr mentioned this pull request Aug 9, 2024

Rollup of 6 pull requests #128879

Closed

bjorn3 reviewed Aug 9, 2024

View reviewed changes

GuillaumeGomez mentioned this pull request Aug 9, 2024

Rollup of 8 pull requests #128881

Closed

matthiaskrgr mentioned this pull request Aug 9, 2024

Rollup of 9 pull requests #128883

Merged

bors merged commit 55329cc into rust-lang:master Aug 9, 2024
6 checks passed

rustbot added this to the 1.82.0 milestone Aug 9, 2024

Nadrieril deleted the is_stolen branch August 10, 2024 10:03

This was referenced Aug 10, 2024

Add an internal lint that warns when accessing untracked data #128919

Merged

Bump rustc version hacspec/hax#838

Merged

Gracefully fallback on stolen bodies AeneasVerif/charon#312

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `Steal::is_stolen()` #128815

Add `Steal::is_stolen()` #128815

Nadrieril commented Aug 8, 2024

rustbot commented Aug 8, 2024

jieyouxu commented Aug 8, 2024

lqd commented Aug 8, 2024

cjgillot commented Aug 8, 2024 •

edited

Loading

lcnr commented Aug 9, 2024

bors commented Aug 9, 2024

lcnr commented Aug 9, 2024

bjorn3 Aug 9, 2024

Nadrieril Aug 9, 2024 •

edited

Loading

bjorn3 Aug 9, 2024

Add Steal::is_stolen() #128815

Add Steal::is_stolen() #128815

Conversation

Nadrieril commented Aug 8, 2024

rustbot commented Aug 8, 2024

jieyouxu commented Aug 8, 2024

lqd commented Aug 8, 2024

cjgillot commented Aug 8, 2024 • edited Loading

lcnr commented Aug 9, 2024

bors commented Aug 9, 2024

lcnr commented Aug 9, 2024

bjorn3 Aug 9, 2024

Choose a reason for hiding this comment

Nadrieril Aug 9, 2024 • edited Loading

Choose a reason for hiding this comment

bjorn3 Aug 9, 2024

Choose a reason for hiding this comment

Add `Steal::is_stolen()` #128815

Add `Steal::is_stolen()` #128815

cjgillot commented Aug 8, 2024 •

edited

Loading

Nadrieril Aug 9, 2024 •

edited

Loading