Skip to content

Apply track_caller to closure on expect_non_local()#96894

Merged
bors merged 1 commit intorust-lang:masterfrom
JohnTitor:expect-non-local-track-caller
Jun 2, 2022
Merged

Apply track_caller to closure on expect_non_local()#96894
bors merged 1 commit intorust-lang:masterfrom
JohnTitor:expect-non-local-track-caller

Conversation

@JohnTitor
Copy link
Member

@JohnTitor JohnTitor commented May 10, 2022

r? @petrochenkov

Alternatively we could remove the closure by replicating the same logic of map_id(). I'm happy to switch to it if you'd like.

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label May 10, 2022
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 10, 2022
@JohnTitor
Copy link
Member Author

JohnTitor commented May 10, 2022

(Will fix the CI failure after discussing a preferred way)

@petrochenkov
Copy link
Contributor

petrochenkov commented May 10, 2022

Using #[track_caller] (and also #[cold]) on the closure would be the better alternative both here and in #96778.

However, something is broken in feature gating of #[track_caller] on closures, and they cannot be "ungated" even if the corresponding #![feature] is in place.
It would be great to find out why it happens and fix that issue.

@petrochenkov petrochenkov 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 May 10, 2022
@JohnTitor
Copy link
Member Author

JohnTitor commented May 10, 2022

Hmm, I'm afraid that I'm not familiar with it. At least, looking at #87064 and related code, feature gating is fine. We use expect_non_local() on rustc_resolve and librustdoc, I think this should be related (actually local build passes when I added the attr on rustc_resolve along with rustc_hir), right? My guess is an attribute (track_caller, in this case) is also added at callsite then a crate containing that callsite will need a feature attr.
However I don't think I can resolve this issue only by my hand, any help would be appreciated.

@petrochenkov
Copy link
Contributor

petrochenkov commented May 10, 2022

cc @Aaron1011

@Aaron1011
Copy link
Contributor

Aaron1011 commented May 11, 2022 •
edited
Loading

This looks like a broader issue - the codegen_fn_attrs query does feature-gating, and used to be called for foreign DefIds. This was recently changed in #96473 - and I confirmed that this issue doesn't occur with ./x.py build --stage 2 src/tools/rustdoc.

For now, I think you could just add #![feature(closure_track_caller)] to rustdoc behind a cfg_attr(bootstrap), and let someone remove it after the next beta bump.

@JohnTitor
Copy link
Member Author

JohnTitor commented May 11, 2022

👍, how about @petrochenkov?

@JohnTitor
Copy link
Member Author

JohnTitor commented May 11, 2022

By the way, is there an issue tracking that weird behavior?

@JohnTitor JohnTitor mentioned this pull request May 16, 2022
Open
@JohnTitor JohnTitor force-pushed the expect-non-local-track-caller branch 2 times, most recently from cba1cf0 to 99d7cc0 Compare May 16, 2022 10:09
@JohnTitor
Copy link
Member Author

JohnTitor commented May 16, 2022

Added the feature attribute to librustdoc as suggested, @petrochenkov could you take a look?

@JohnTitor JohnTitor 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 May 16, 2022
@petrochenkov
Copy link
Contributor

petrochenkov commented May 16, 2022

Maybe we should just wait until #96473 gets into the bootstrap compiler, it's going to happen pretty soon.
Adding the feature to all crates is not scalable, e.g. for #96778 that would mean adding it to majority of compiler crates.

@petrochenkov petrochenkov added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 16, 2022
@petrochenkov
Copy link
Contributor

petrochenkov commented May 23, 2022

Blocked on #97214.

@petrochenkov
Copy link
Contributor

petrochenkov commented May 29, 2022

#97214 has been merged.
@rustbot author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-blocked Status: Blocked on something else such as an RFC or other implementation work. labels May 29, 2022
@JohnTitor JohnTitor force-pushed the expect-non-local-track-caller branch from 99d7cc0 to c7db4b0 Compare May 31, 2022 14:57
@petrochenkov
Copy link
Contributor

petrochenkov commented May 31, 2022

Thanks!
@bors r+ rollup

@bors
Copy link
Collaborator

bors commented May 31, 2022

📌 Commit c7db4b0 has been approved by petrochenkov

@bors bors removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label May 31, 2022
@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label May 31, 2022
@JohnTitor
Copy link
Member Author

JohnTitor commented Jun 2, 2022

The merge queue doesn't have this PR, let me close/reopen to fix it...

@JohnTitor JohnTitor closed this Jun 2, 2022
@JohnTitor JohnTitor reopened this Jun 2, 2022
@Dylan-DPC Dylan-DPC mentioned this pull request Jun 2, 2022
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 2, 2022
@bors
Auto merge of rust-lang#97644 - Dylan-DPC:rollup-xaeio91, r=Dylan-DPC
9598b4b 
Rollup of 6 pull requests

Successful merges:

 - rust-lang#96894 (Apply track_caller to closure on `expect_non_local()`)
 - rust-lang#97023 (Diagnose anonymous lifetimes errors more uniformly between async and regular fns)
 - rust-lang#97397 (Stabilize `box_into_pin`)
 - rust-lang#97587 (Migrate more diagnostics to use the `#[derive(SessionDiagnostic)]`)
 - rust-lang#97603 (Arc make_mut doc comment spelling correction.)
 - rust-lang#97635 (Fix file metadata documentation for Windows)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit ddc5d2c into rust-lang:master Jun 2, 2022
@rustbot rustbot added this to the 1.63.0 milestone Jun 2, 2022
@JohnTitor JohnTitor deleted the expect-non-local-track-caller branch June 2, 2022 22:03
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

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.63.0

Development

Successfully merging this pull request may close these issues.

6 participants

@JohnTitor @petrochenkov @Aaron1011 @bors @rust-highfive @rustbot