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

Latest nightly on edition 2024 results in compilation failure #434

Closed
indietyp opened this issue Nov 4, 2024 · 10 comments · Fixed by #435
Closed

Latest nightly on edition 2024 results in compilation failure #434

indietyp opened this issue Nov 4, 2024 · 10 comments · Fixed by #435
Labels
bug Something isn't working

Comments

@indietyp
Copy link
Contributor

indietyp commented Nov 4, 2024

The derive code generate by logos no longer compiles for edition 2024 on nightly-2024-11-04, compilation now results in the error:

     |
  10 | #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Logos)]
     |                                                              ^^^^^
     |                                                              |
     |                                                              `*lex` was mutably borrowed here in the previous iteration of the loop
     |                                                              first borrow later used by call
     |
  note: this call may capture more lifetimes than intended, because Rust 2024 has adjusted the `impl Trait` lifetime capture rules
    --> libs/@local/hql/syntax-jexpr/src/lexer/token_kind.rs:10:62
     |
  10 | #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Logos)]
     |                                                              ^^^^^
     = note: this error originates in the derive macro `Logos` (in Nightly builds, run with -Z macro-backtrace for more info)
  help: add a precise capturing bound to avoid overcapturing
     |
  10 | #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Logos + use<'s>)]
     |                                                                    +++++++++

The code resulting in the issue:

https://github.com/hashintel/hash/blob/fb84b5182d252203c5fc58eac6a68e67eb1144c7/libs/%40local/hql/syntax-jexpr/src/lexer/token_kind.rs#L10-L45

@jeertmans
Copy link
Collaborator

Hello @indietyp, thank you for reporting!

Nightly isn’t stable, so I not rush before calling that a bug. Could you find the RFC or nightly feature that breaks the build? So I can read the details and see how likely it is going to break our code in the future.

@indietyp
Copy link
Contributor Author

indietyp commented Nov 10, 2024

Yea sure, this has something to do with edition 2024, in which the impl capture rules are changed, see: rust-lang/rust#117587 and rust-lang/rfcs#3498

This has regressed between 2024-10-28 and 2024-11-04.

Specifically in our example, the problem was with:

fn callback<'s>(_: &mut Lexer<'s>) -> impl CallbackResult<'s, bool, TokenKind<'s>> {
    false
}

adding a + use<'s> will solve this at the return type will be forced to only capture s and not the anonymous lifetime.

In edition 2024 this captures both 's and the anonymous lifetime of the lexer. This is intended behaviour for edition 2024 and onwards. (see RFC)

These callback functions are generated whenever someone specifies an inline callback closure, in our example this is:

#[token("false", |_| false)]
#[token("true", |_| true)]
Bool(bool),

@jeertmans jeertmans added the bug Something isn't working label Nov 12, 2024
@jeertmans
Copy link
Collaborator

Ok, I see, thanks for the links!
If I understand correctly, this will be part of edition 2024, so no doubt we have to address this issue.

Would you mind making a PR?

@indietyp
Copy link
Contributor Author

I don't mind, it seems to be a relatively small change, the only problem I see is in detecting if someone is on edition 2024, any idea on how one might detect that?

@jeertmans
Copy link
Collaborator

jeertmans commented Nov 12, 2024

Ah this uses a new syntax that wasn't available before?

If so, that means we will either have code-compatible prior to 2024, or post 2024, but not both, unless we use a third-party crate like rustversion. I am not completely against this solution, but I prefer avoiding adding new deps, especially proc macros. However, I don't think the standard library offers an alternative (but we could probably use some build.rs file that emits the rust edition).

@indietyp
Copy link
Contributor Author

indietyp commented Nov 12, 2024

preciese_capturing (which is the feature) has been stabilized in rust 1.82 - regardless of edition - so in theory one could gate the use of use behind the compiler used. That should be possible through a build script, tho I don't know how build scripts interact with proc macros.

In theory adding a build.rs and doing something similar to what error-stack is doing could be the solution:

https://github.com/hashintel/hash/blob/9bad74369c3804eae32df06aecf9fb0fd3cddb21/libs/error-stack/build.rs#L11-L23.

I'd be willing to give that a try if you're okay with that, as that means we won't need to increase the MSRV.

The std offers cfg_version, but that's still unstable.

@jeertmans
Copy link
Collaborator

Yes, your suggestion looks great!

@Tebarem
Copy link

Tebarem commented Dec 1, 2024

Hey Im on 2024 and im Getting this Issue. I see you created and merged a PR. Any ETA on when a new version will comeout?

@jeertmans
Copy link
Collaborator

jeertmans commented Dec 2, 2024

Hey Im on 2024 and im Getting this Issue. I see you created and merged a PR. Any ETA on when a new version will comeout?

Just released now, @Tebarem! See v0.14.3 :-)

@Tebarem
Copy link

Tebarem commented Dec 2, 2024

GOAT

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants