Allow to customize // TODO: comment for deprecated safe autofix
#127857
+49
−13
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rustbot
added
S-waiting-on-review
This comment has been minimized.
This comment has been minimized.
tbu-
force-pushed
the
pr_deprecated_safe_todo
branch
from
b2e98da to
44d3664
Compare
This comment has been minimized.
This comment has been minimized.
tbu-
force-pushed
the
pr_deprecated_safe_todo
branch
from
44d3664 to
5502eec
Compare
This comment has been minimized.
This comment has been minimized.
tbu-
force-pushed
the
pr_deprecated_safe_todo
branch
from
5502eec to
c719526
Compare
This comment has been minimized.
This comment has been minimized.
tbu-
force-pushed
the
pr_deprecated_safe_todo
branch
from
c719526 to
bf612e9
Compare
RalfJung
reviewed
Jul 17, 2024
5 tasks
This comment was marked as resolved.
This comment was marked as resolved.
tbu-
force-pushed
the
pr_deprecated_safe_todo
branch
from
bf612e9 to
a253f11
Compare
|
We haven't heard from the reviewer in 2 weeks and this is edition-critical. r? compiler |
| fn parse_rustc_deprecated_safe_2024_attr(attr: &Attribute) -> Option<Symbol> { | ||
| for item in attr.meta_item_list().unwrap_or_default() { | ||
| if item.has_name(sym::audit_that) { | ||
| return Some(item.value_str().expect( |
There was a problem hiding this comment.
Suggested change
| return Some(item.value_str().expect( | |
| if let Some(msg) = item.value_str() { |
The current code will most likely ICE if the value is not actually string.
There was a problem hiding this comment.
That sounds better to me than silently ignoring the attribute value in that case. It's an internal attribute after all. I've seen the pattern of abusing ICE instead of generating proper errors in other internal attributes as well.
There was a problem hiding this comment.
Not a good practice, but not a blocker.
There was a problem hiding this comment.
I have verified that this indeed ICEs. I'd still prefer the ICE over silently ignoring the attribute value. The compiler error says:
error: the compiler unexpectedly panicked. this is a bug.
note: using internal features is not supported and expected to cause internal compiler errors when used incorrectly
petrochenkov
added
S-waiting-on-author
tbu-
force-pushed
the
pr_deprecated_safe_todo
branch
from
445595b to
9ad9819
Compare
|
@rustbot ready |
rustbot
added
S-waiting-on-review
petrochenkov
added
S-waiting-on-author
Relevant for the deprecation of `CommandExt::before_exit` in rust-lang#125970.
…er error This doesn't work for translated compiler error messages.
tbu-
force-pushed
the
pr_deprecated_safe_todo
branch
from
9ad9819 to
811d7dd
Compare
|
@rustbot ready |
rustbot
added
S-waiting-on-review
|
@bors r+ |
bors
added
S-waiting-on-bors
GuillaumeGomez
added a commit
to GuillaumeGomez/rust
that referenced
this pull request
Aug 13, 2024
…trochenkov Allow to customize `// TODO:` comment for deprecated safe autofix Relevant for the deprecation of `CommandExt::before_exit` in rust-lang#125970. Tracking: - rust-lang#124866
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Aug 13, 2024
…llaumeGomez Rollup of 7 pull requests Successful merges: - rust-lang#127857 (Allow to customize `// TODO:` comment for deprecated safe autofix) - rust-lang#128410 (Migrate `remap-path-prefix-dwarf` `run-make` test to rmake) - rust-lang#128828 (`-Znext-solver` caching) - rust-lang#128873 (Add windows-targets crate to std's sysroot) - rust-lang#129034 (Add `#[must_use]` attribute to `Coroutine` trait) - rust-lang#129049 (compiletest: Don't panic on unknown JSON-like output lines) - rust-lang#129050 (Emit a warning instead of an error if `--generate-link-to-definition` is used with other output formats than HTML) r? `@ghost` `@rustbot` modify labels: rollup
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Aug 13, 2024
…iaskrgr Rollup of 7 pull requests Successful merges: - rust-lang#122884 (Optimize integer `pow` by removing the exit branch) - rust-lang#127857 (Allow to customize `// TODO:` comment for deprecated safe autofix) - rust-lang#129034 (Add `#[must_use]` attribute to `Coroutine` trait) - rust-lang#129049 (compiletest: Don't panic on unknown JSON-like output lines) - rust-lang#129050 (Emit a warning instead of an error if `--generate-link-to-definition` is used with other output formats than HTML) - rust-lang#129056 (Fix one usage of target triple in bootstrap) - rust-lang#129058 (Add mw back to review rotation) r? `@ghost` `@rustbot` modify labels: rollup
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Aug 14, 2024
…iaskrgr Rollup of 7 pull requests Successful merges: - rust-lang#122884 (Optimize integer `pow` by removing the exit branch) - rust-lang#127857 (Allow to customize `// TODO:` comment for deprecated safe autofix) - rust-lang#129034 (Add `#[must_use]` attribute to `Coroutine` trait) - rust-lang#129049 (compiletest: Don't panic on unknown JSON-like output lines) - rust-lang#129050 (Emit a warning instead of an error if `--generate-link-to-definition` is used with other output formats than HTML) - rust-lang#129056 (Fix one usage of target triple in bootstrap) - rust-lang#129058 (Add mw back to review rotation) r? `@ghost` `@rustbot` modify labels: rollup
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Aug 14, 2024
…iaskrgr Rollup of 7 pull requests Successful merges: - rust-lang#122884 (Optimize integer `pow` by removing the exit branch) - rust-lang#127857 (Allow to customize `// TODO:` comment for deprecated safe autofix) - rust-lang#129034 (Add `#[must_use]` attribute to `Coroutine` trait) - rust-lang#129049 (compiletest: Don't panic on unknown JSON-like output lines) - rust-lang#129050 (Emit a warning instead of an error if `--generate-link-to-definition` is used with other output formats than HTML) - rust-lang#129056 (Fix one usage of target triple in bootstrap) - rust-lang#129058 (Add mw back to review rotation) r? `@ghost` `@rustbot` modify labels: rollup
rust-timer
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Aug 14, 2024
Rollup merge of rust-lang#127857 - tbu-:pr_deprecated_safe_todo, r=petrochenkov Allow to customize `// TODO:` comment for deprecated safe autofix Relevant for the deprecation of `CommandExt::before_exit` in rust-lang#125970. Tracking: - rust-lang#124866
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Relevant for the deprecation of
CommandExt::before_exitin #125970.Tracking:
std::env::{set_var, remove_var}unsafe #124866