udoprog / genco

A whitespace-aware quasiquoter for beautiful code generation.
Apache License 2.0
181 stars 11 forks source link

`proc-macro2@1.0.56` breaks `quote!` line formatting #39

Closed obrok closed 11 months ago

obrok commented 1 year ago

It seems that upgrading to proc-macro2 >=1.0.56 breaks how newlines are handled in the generated code.

Steps to reproduce:

  1. cargo update -p proc-macro2 --precise 1.0.56
  2. cargo test

Note that it seems to work correctly for 1.0.55.

udoprog commented 1 year ago

Thanks for the report!

This is due to dtolnay/proc-macro2#383 which is in preparation of a maybe breaking nightly upcoming change.

Seems like things are gonna be broken for a while. Welcome to nightly :)

obrok commented 1 year ago

I locked to an older proc-macro2 for now in my package... Do you think maybe you should specify proc-macro2 <=1.0.55 in your deps until this is fixed over there?

udoprog commented 1 year ago

Hm... maybe. It's going to instantly break if / once that nightly change lands, so I'm not sure there's a ton of benefit to be had by making a release right now. I'll have to do one anyway once the new API is available. I'll think about it.

udoprog commented 1 year ago

We might also soon see stable support for line/column information, meaning the years long wait would be over :tada:

https://github.com/rust-lang/rust/issues/54725#issuecomment-1546918161

obrok commented 1 year ago

Well, difficult for me to weigh in, because I don't have a good grasp on the timescales involved. Thanks for the explanation though.

udoprog commented 1 year ago

I'll bake a new release if https://github.com/rust-lang/rust/pull/111571 is not out on nightly within the week. Sorry for the inconvenience. Keeping this open to track the issue.

obrok commented 1 year ago

Not a problem and thanks for the quick responses!

kklas commented 1 year ago

Also affected by this. Can't pin proc-macro2 to 1.0.55 because other dependencies I'm using require a higher version.

udoprog commented 1 year ago

All right. Time to start building a workaround, we'll have to bypass proc-macro2 for now - because the API there has been stripped before the new nightly API is available. I'll see what I can do.

udoprog commented 1 year ago

Also affected by this. Can't pin proc-macro2 to 1.0.55 because other dependencies I'm using require a higher version.

@kklas I'm curious; Which other dependency?

kklas commented 1 year ago

I'm building a code generator tool against https://github.com/MystenLabs/sui. It's a huge repo pulling in a lot of transitive dependencies. This is what I get trying to pin proc-macro2 to 1.0.55:

$ cargo update -p proc-macro2@1.0.59 --precise 1.0.55
    Updating crates.io index
error: failed to select a version for the requirement `proc-macro2 = "^1.0.59"`
candidate versions found which didn't match: 1.0.55
location searched: crates.io index
required by package `syn v2.0.18`
    ... which satisfies dependency `syn = "^2.0.9"` (locked to 2.0.18) of package `async-trait v0.1.68`
    ... which satisfies dependency `async-trait = "^0.1.57"` (locked to 0.1.68) of package `anemo v0.0.0 (https://github.com/mystenlabs/anemo.git?rev=baa7cfc3ae841f88c2ff773396c5a803c377d054#baa7cfc3)`
    ... which satisfies git dependency `anemo` (locked to 0.0.0) of package `anemo-cli v0.0.0 (https://github.com/mystenlabs/anemo.git?rev=baa7cfc3ae841f88c2ff773396c5a803c377d054#baa7cfc3)`
    ... which satisfies git dependency `anemo-cli` (locked to 0.0.0) of package `workspace-hack v0.1.0 (https://github.com/MystenLabs/sui?branch=releases/sui-v1.2.0-release#5938864a)`
    ... which satisfies git dependency `workspace-hack` (locked to 0.1.0) of package `mysten-metrics v0.7.0 (https://github.com/MystenLabs/sui?branch=releases/sui-v1.2.0-release#5938864a)`
    ... which satisfies git dependency `mysten-metrics` (locked to 0.7.0) of package `sui-adapter-latest v0.1.0 (https://github.com/MystenLabs/sui?branch=releases/sui-v1.2.0-release#5938864a)`
    ... which satisfies git dependency `sui-adapter` (locked to 0.1.0) of package `sui-move-build v0.0.0 (https://github.com/MystenLabs/sui?branch=releases/sui-v1.2.0-release#5938864a)`
    ... which satisfies git dependency `sui-move-build` (locked to 0.0.0) of package `generator v0.1.0 (/home/user/work/sui-client-gen/generator)`
perhaps a crate was updated and forgotten to be re-vendored?

Even if I could fix this (which I can't) there would probably be others.

udoprog commented 1 year ago

Ah, definitely not, it's syn2 which has bumped the dependency: https://github.com/dtolnay/syn/commit/ee29399c5f10bbfe804d9f09a6022b9596dba53e

I don't think there's a way for me to fix this other than wait for a new nightly and proc-macro2 to update. There's no APIs to access the underlying Span and proc-macro2 is a deep dependency for all macro processing (parsing in syn which is used in this crate depends on it).

kklas commented 1 year ago

Ok, thanks for taking a look. I will work around it in the meantime by throwing a formatter at it after the code is generated. I hope these APIs you need land in stable soon so we don't have to use nightly.

udoprog commented 1 year ago

Me too, especially since I no longer can use nightly!

udoprog commented 1 year ago

Since this has delayed for so long, a nightly only workaround is now available. By adding the following patches:

[patch.crates-io]
genco = { git = "https://github.com/udoprog/genco", branch = "proc-macro2-fork" }
proc-macro2 = { git = "https://github.com/udoprog/proc-macro2", branch = "span-locations" }
obrok commented 11 months ago

Unfortunately, this does not seem to work. I do:

cargo new test-genco

then in Cargo.toml:

[dependencies]
genco = { git = "https://github.com/udoprog/genco", branch = "proc-macro2-fork" }

[patch.crates-io]
proc-macro2 = { git = "https://github.com/udoprog/proc-macro2", branch = "span-locations" }

then cargo build gives:

error[E0308]: mismatched types
   --> /home/yapee/.cargo/git/checkouts/proc-macro2-c2481842691181e5/77564cb/src/wrapper.rs:449:49
    |
449 |             Self::Compiler(s) => Self::Compiler(s.start()),
    |                                  -------------- ^^^^^^^^^ expected `Span`, found `LineColumn`
    |                                  |
    |                                  arguments to this enum variant are incorrect

and some more errors.


Edit: same with

proc-macro2 = { git = "https://github.com/hydro-project/proc-macro2", branch = "new-span-locs" }

which is the actual version currently set in the genco repo.

obrok commented 10 months ago

Hi, I can confirm this works, so thanks for the fix! However, it seems a sufficiently-recent nightly is needed? I set nightly-2023-09-25 and it works - maybe you could set some min rust version in Cargo.toml? In any case - thanks again.

udoprog commented 10 months ago

Hm, since there's no nightly-specific rust version I can set in Cargo.toml I'm a bit hesitant to pin the project on stable to that MSRV.

Besides, the feature might change in a future nightly again, so that declaration would not be accurate. For now I'd recommend the latest nightly until its stabilized. It might however be worth noting somewhere in the README that it requires at least a a 1.73 nightly or beyond. That's where I think the feature was changed with Span::start / Span::end.

Thanks for pointing this out to me! It's well worth considering.