Skip to content

Conversation

FabianWolff
Copy link
Contributor

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 26, 2021
@LeSeulArtichaut
Copy link
Contributor

LeSeulArtichaut commented Jul 26, 2021

AFAICT this PR introduces a correctness hole:

unsafe fn unsf() {}
fn bad<T>() -> Box<dyn Iterator<Item = [(); { unsafe { || { unsf() } }; 4 }]>> { todo!() }
//                                                          ^^^^^^ shouldn't error, but does with this PR

I think the issue in #87414 is rather that the closure tries to run unsafeck for bad instead of bad::{constant#0}, which means that this is incorrect:

let owner = tcx.hir().local_def_id_to_hir_id(def.did).owner;

This works for closures in functions but apparently misbehaves in constants. The proper fix is probably changing this to use enclosing_body_owner.

@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-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 26, 2021
@rustbot
Copy link
Collaborator

rustbot commented Jul 26, 2021

@FabianWolff

@FabianWolff
Copy link
Contributor Author

Good point; this should be fixed now (I've also expanded the test case with your example).

@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 Jul 27, 2021
@rustbot
Copy link
Collaborator

rustbot commented Jul 27, 2021

@LeSeulArtichaut

@rust-log-analyzer

This comment has been minimized.

@LeSeulArtichaut
Copy link
Contributor

Sorry if I explained badly, I was wondering if using enclosing_body_owner instead of the HirId owner hack was enough to fix the issue, without having special casing on AnonConsts. Can you try that?

@FabianWolff
Copy link
Contributor Author

Superseded by #87645.

JohnTitor added a commit to JohnTitor/rust that referenced this pull request Aug 3, 2021
Properly find owner of closure in THIR unsafeck

Previously, when encountering a closure in a constant, the THIR unsafeck gets invoked on the owner of the constant instead of the constant itself, producing cycles.
Supersedes rust-lang#87492. `@FabianWolff` thanks for your work on that PR, I copied your test file and added you as a co-author.

Fixes rust-lang#87414.
r? `@oli-obk`
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Aug 3, 2021
Properly find owner of closure in THIR unsafeck

Previously, when encountering a closure in a constant, the THIR unsafeck gets invoked on the owner of the constant instead of the constant itself, producing cycles.
Supersedes rust-lang#87492. ``@FabianWolff`` thanks for your work on that PR, I copied your test file and added you as a co-author.

Fixes rust-lang#87414.
r? ``@oli-obk``
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Aug 3, 2021
Properly find owner of closure in THIR unsafeck

Previously, when encountering a closure in a constant, the THIR unsafeck gets invoked on the owner of the constant instead of the constant itself, producing cycles.
Supersedes rust-lang#87492. ```@FabianWolff``` thanks for your work on that PR, I copied your test file and added you as a co-author.

Fixes rust-lang#87414.
r? ```@oli-obk```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

thir-unsafeck query loops on closures in const-generics
5 participants