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

compilation error in der_derive #1617

Open
rscarson opened this issue Dec 3, 2024 · 12 comments
Open

compilation error in der_derive #1617

rscarson opened this issue Dec 3, 2024 · 12 comments

Comments

@rscarson
Copy link

rscarson commented Dec 3, 2024

This crate no longer compiles on the newest versions of rustic

Builds fine on 1.81, but nightly 1.85 has the following:

error[E0502]: cannot borrow `generics.params` as mutable because it is also borrowed as immutable
  --> der_derive-0.7.3\src\sequence.rs:61:29
   |
57 |           let lifetime = generics
   |                          --------
   |                          |
   |  ________________________immutable borrow occurs here
   | |
58 | |             .lifetimes()
   | |________________________- a temporary with access to the immutable borrow is created here ...
...
61 |               .unwrap_or_else(|| {
   |                               ^^ mutable borrow occurs here
62 |                   let lt = default_lifetime();
63 | /                 generics
64 | |                     .params
   | |___________________________- second borrow occurs due to use of `generics.params` in closure
...
67 |               });
   |                 - ... and the immutable borrow might be used here, when that temporary is dropped and runs the destructor for type `impl Iterator<Item = &LifetimeParam>`
@tarcieri
Copy link
Member

tarcieri commented Dec 3, 2024

That sounds like a nightly regression. I would suggest searching the Rust issue tracker.

We can potentially patch around it but really if something compiles on stable today, it's supposed to compile in any future releases, because otherwise it would be a breaking change and stable is supposed to be free of those.

@tarcieri
Copy link
Member

tarcieri commented Dec 3, 2024

Note: seems quite similar to this: dtolnay/syn#1718

@rscarson
Copy link
Author

rscarson commented Dec 3, 2024

Seems so! Very very odd

A regression in syn then?

@rscarson
Copy link
Author

rscarson commented Dec 4, 2024

So it compiles on deno/main, using syn 2.0.87

But not on rustyscript/master, with syn 2.0.89, even if I lock the version to .87 there too

However, removing --cfg=docsrs fixes the issue, even on the same nightly compiler

And I see that syn uses cfg_attr(docsrs extensively - there is likely an issue there

@rscarson
Copy link
Author

rscarson commented Dec 4, 2024

Appears syn has no plans to fix it

@tarcieri
Copy link
Member

tarcieri commented Dec 4, 2024

syn fixed the linked issue (1718), because that was a breaking API change.

It seems like potentially there has been a regression in rustc with a similar root cause: the borrow checker assuming it needs a mutable reference to run drop glue, when there is no Drop impl on the relevant types

@tarcieri
Copy link
Member

tarcieri commented Dec 4, 2024

Also I should say we can change the code to use match instead of combinators, since this seems to be some code whose lifetimes are weird enough to have broken both syn and rustc itself

@tarcieri
Copy link
Member

tarcieri commented Dec 5, 2024

However, removing --cfg=docsrs fixes the issue, even on the same nightly compiler

I'm not sure what's happening there but it seems this compiles under nightly without this setting? If so, I'm not sure what's actionable here

@rscarson
Copy link
Author

rscarson commented Dec 5, 2024

However, removing --cfg=docsrs fixes the issue, even on the same nightly compiler

I'm not sure what's happening there but it seems this compiles under nightly without this setting? If so, I'm not sure what's actionable here

I mean
The only downside is I can't test my documentation locally anymore

It's very odd, if nothing else

@rscarson
Copy link
Author

rscarson commented Dec 5, 2024

I would also argue it may be worth a refactor if it has confused the borrow checker on at least 2 occasions

@rscarson
Copy link
Author

rscarson commented Dec 5, 2024

I found the cause

This macro: https://github.com/dtolnay/syn/blob/91ffc5c4c53e1b3ab961dd3bd7e3fcf79d15cb7c/src/macros.rs#L174

Called here: https://github.com/dtolnay/syn/blob/91ffc5c4c53e1b3ab961dd3bd7e3fcf79d15cb7c/src/generics.rs#L106

Replaces the return type if docsrs is on. In this case to an iterator impl

@Nemo157
Copy link

Nemo157 commented Dec 5, 2024

The only downside is I can't test my documentation locally anymore

The general approach to testing "like docs.rs" is something like cargo rustdoc -- --cfg=docsrs (which does not pass --cfg=docsrs when building dependencies) or using cargo docs-rs which handles a lot more of the customizations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants