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

error message found by NLL in librustc_codegen_llvm #53221

Closed
nikomatsakis opened this issue Aug 9, 2018 · 5 comments
Closed

error message found by NLL in librustc_codegen_llvm #53221

nikomatsakis opened this issue Aug 9, 2018 · 5 comments
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. A-NLL Area: Non Lexical Lifetimes (NLL) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Milestone

Comments

@nikomatsakis
Copy link
Contributor

Trying to port librustc_codegen_llvm to use NLL (cc #53172) yields these errors:

https://gist.github.com/memoryruins/14a2aad7fc85d0429ae9e4240ec0dacb

The error seem legitimate. I reduced the pattern in question to this test case:

https://play.rust-lang.org/?gist=6e22b15051f9a108d09f28a0b657f717&version=nightly&mode=debug&edition=2015

As far as I can tell, the Some(child.raw) is interpreted as a Some(&mut *child.raw), which borrows from Child -- but that borrow outlists the variable child. And child has a dtor that frees memory. So something may be legitimately wrong here?

You can solve this by adding a take method that clears the raw field afterwards:

https://play.rust-lang.org/?gist=64c20d529ad49d5871417c391a8dbcde&version=nightly&mode=debug&edition=2015

But I'm not sure what is the intention.

cc @eddyb @irinagpopa

@nikomatsakis nikomatsakis added A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. A-NLL Area: Non Lexical Lifetimes (NLL) WG-compiler-nll labels Aug 9, 2018
@nikomatsakis nikomatsakis added this to the Rust 2018 RC milestone Aug 9, 2018
@nikomatsakis
Copy link
Contributor Author

Putting on the 2018 RC since this blocks #53172.

@nikomatsakis
Copy link
Contributor Author

Here is the code that is failing to compile:

for child in archive.iter() {
let child = child.map_err(string_to_io_error)?;
let child_name = match child.name() {
Some(s) => s,
None => continue,
};
if removals.iter().any(|r| r == child_name) {
continue
}
let name = CString::new(child_name)?;
members.push(llvm::LLVMRustArchiveMemberNew(ptr::null(),
name.as_ptr(),
Some(child.raw)));
strings.push(name);
}

@eddyb
Copy link
Member

eddyb commented Aug 9, 2018

What happens if this line is changed from &'a ArchiveChild to &ArchiveChild<'a>?

Child: Option<&'a ArchiveChild>)

An &ArchiveChild doesn't point into the archive itself, it points to an owned object that itself points to the archive, and LLVMRustArchiveMemberNew copies the ArchiveChild (whereas the current signature suggests it keeps the &ArchiveChild).

memoryruins added a commit to memoryruins/rust that referenced this issue Aug 10, 2018
…ted out by nll

As explained by eddyb in rust-lang#53221, "An &ArchiveChild doesn't point into the archive itself, it points to an owned object that itself points to the archive, and LLVMRustArchiveMemberNew copies the ArchiveChild (whereas the current signature suggests it keeps the &ArchiveChild)."
@memoryruins
Copy link
Contributor

@eddyb your suggestion was on point :)
fixed in commit: 085535b
merged in rollup: #53297

@nikomatsakis
Copy link
Contributor Author

I'll close this then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. A-NLL Area: Non Lexical Lifetimes (NLL) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

3 participants