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

Switch #[track_caller] back to a no-op unless feature gate is enabled #104741

Merged
merged 7 commits into from
Dec 22, 2022

Conversation

bryangarza
Copy link
Contributor

This patch fixes a regression, in which #[track_caller], which was previously a no-op, was changed to actually turn on the behavior. This should instead only happen behind the closure_track_caller feature gate.

Also, add a warning for the user to understand how their code will compile depending on the feature gate being turned on or not.

Fixes #104588

@rustbot
Copy link
Collaborator

rustbot commented Nov 22, 2022

r? @jackh726

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added A-translation Area: Translation infrastructure, and migrating existing diagnostics to SessionDiagnostic 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 22, 2022
@rustbot
Copy link
Collaborator

rustbot commented Nov 22, 2022

rustc_error_messages was changed

cc @davidtwco, @compiler-errors, @JohnTitor, @estebank, @TaKO8Ki

@bryangarza
Copy link
Contributor Author

r? @compiler-errors

@rustbot rustbot assigned compiler-errors and unassigned jackh726 Nov 22, 2022
@bryangarza bryangarza force-pushed the bug-104588-async-track-caller branch 2 times, most recently from 7d748cd to e589c06 Compare November 22, 2022 21:43
@rust-log-analyzer

This comment has been minimized.

Copy link
Member

@compiler-errors compiler-errors left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR. I had some questions, though I may be missing some context, so feel free to correct me if I assumed something incorrectly.

compiler/rustc_ast_lowering/src/expr.rs Outdated Show resolved Hide resolved
compiler/rustc_ast_lowering/src/expr.rs Outdated Show resolved Hide resolved
compiler/rustc_ast_lowering/src/expr.rs Outdated Show resolved Hide resolved
compiler/rustc_error_messages/locales/en-US/lint.ftl Outdated Show resolved Hide resolved
compiler/rustc_lint/src/builtin.rs Outdated Show resolved Hide resolved
compiler/rustc_lint/src/builtin.rs Outdated Show resolved Hide resolved
@bryangarza
Copy link
Contributor Author

bryangarza commented Nov 24, 2022

I addressed most of the comments and resolved their threads, but there's a couple more I have to follow up with in another commit :)

@bryangarza
Copy link
Contributor Author

bryangarza commented Nov 24, 2022

  • TODO: add a doc comment to the lint definition

@rust-log-analyzer

This comment has been minimized.

@compiler-errors compiler-errors 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-review Status: Awaiting review from the assignee but also interested parties. labels Nov 25, 2022
@bors
Copy link
Contributor

bors commented Dec 2, 2022

☔ The latest upstream changes (presumably #104863) made this pull request unmergeable. Please resolve the merge conflicts.

@bryangarza
Copy link
Contributor Author

My latest commit is near-identical to #105180 so I think I will need to just remove that one and rebase (there might be a couple lines to keep from the commit, but not related to the hirId)

@bryangarza
Copy link
Contributor Author

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 7, 2022
compiler/rustc_ast_lowering/src/expr.rs Outdated Show resolved Hide resolved
compiler/rustc_lint/src/builtin.rs Outdated Show resolved Hide resolved
compiler/rustc_lint/src/builtin.rs Outdated Show resolved Hide resolved
compiler/rustc_lint/src/builtin.rs Outdated Show resolved Hide resolved
@bryangarza
Copy link
Contributor Author

bryangarza commented Dec 13, 2022

Thanks for the comments! I’ll update this PR soon

@bors
Copy link
Contributor

bors commented Dec 14, 2022

☔ The latest upstream changes (presumably #105667) made this pull request unmergeable. Please resolve the merge conflicts.

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 21, 2022
@compiler-errors
Copy link
Member

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Dec 22, 2022

📌 Commit ccbba0a has been approved by compiler-errors

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 Dec 22, 2022
@compiler-errors compiler-errors added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Dec 22, 2022
@compiler-errors
Copy link
Member

compiler-errors commented Dec 22, 2022

Beta nominating this because this fixes a regression that's currently stable-to-beta (introduced in #104219) where #[track_caller] on async fn turned from a no-op to a hard error. This instead emits a warning in that case for future compatibility reasons, but still allows the code to compile with the track_caller as a no-op.

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 22, 2022
…caller, r=compiler-errors

Switch `#[track_caller]` back to a no-op unless feature gate is enabled

This patch fixes a regression, in which `#[track_caller]`, which was previously a no-op, was changed to actually turn on the behavior. This should instead only happen behind the `closure_track_caller` feature gate.

Also, add a warning for the user to understand how their code will compile depending on the feature gate being turned on or not.

Fixes rust-lang#104588
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 22, 2022
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#104741 (Switch `#[track_caller]` back to a no-op unless feature gate is enabled)
 - rust-lang#105769 (add function to tell the identical errors for ambiguity_errors)
 - rust-lang#105843 (Suggest associated const on possible capitalization mistake)
 - rust-lang#105966 (Re-enable `Fn` trait call notation error for non-tuple argument)
 - rust-lang#106002 (codegen tests: adapt patterns to also work with v0 symbol mangling)
 - rust-lang#106010 (Give opaque types a better coherence error)
 - rust-lang#106016 (rustdoc: simplify link anchor to section expand JS)
 - rust-lang#106024 (Fix ICE due to `todo!()` in `rustdoc` for `Term`s)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 0adf9e0 into rust-lang:master Dec 22, 2022
@rustbot rustbot added this to the 1.68.0 milestone Dec 22, 2022
@apiraino
Copy link
Contributor

Beta backport approved as per compiler team on Zulip

@rustbot label +beta-accepted

@rustbot rustbot added the beta-accepted Accepted for backporting to the compiler in the beta channel. label Dec 22, 2022
@bryangarza bryangarza deleted the bug-104588-async-track-caller branch December 22, 2022 22:22
@Mark-Simulacrum
Copy link
Member

This PR does not backport cleanly -- @bryangarza would you be able to prepare a PR targeting the beta branch?

@bryangarza
Copy link
Contributor Author

Sounds good, I'll create the backport PR

@bryangarza bryangarza restored the bug-104588-async-track-caller branch December 27, 2022 17:20
@Mark-Simulacrum Mark-Simulacrum modified the milestones: 1.68.0, 1.67.0 Dec 27, 2022
@Mark-Simulacrum Mark-Simulacrum removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Dec 27, 2022
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 28, 2022
…ller_beta-backport, r=compiler-errors

Backport: Switch `#[track_caller]` back to a no-op unless feature gate is enabled

Backport of rust-lang#104741

r? `@compiler-errors` since you reviewed the original PR
requested by `@Mark-Simulacrum` rust-lang#104741 (comment)
@bryangarza bryangarza deleted the bug-104588-async-track-caller branch December 28, 2022 03:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-translation Area: Translation infrastructure, and migrating existing diagnostics to SessionDiagnostic 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
Development

Successfully merging this pull request may close these issues.

#[track_caller] on async fn's was previously a no-op but now produces a compiler error
9 participants