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

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

Closed
obrok opened this issue May 22, 2023 · 18 comments
Closed

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

obrok opened this issue May 22, 2023 · 18 comments
Labels

Comments

@obrok
Copy link

obrok commented May 22, 2023

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
Copy link
Owner

udoprog commented May 22, 2023

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 :)

@udoprog udoprog added the bug label May 22, 2023
@obrok
Copy link
Author

obrok commented May 22, 2023

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
Copy link
Owner

udoprog commented May 22, 2023

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
Copy link
Owner

udoprog commented May 22, 2023

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

rust-lang/rust#54725 (comment)

@obrok
Copy link
Author

obrok commented May 22, 2023

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

obrok added a commit to Cardinal-Cryptography/ink-wrapper that referenced this issue May 22, 2023
proc-macro2 breaks genco (see
udoprog/genco#39). For now, we can introduce a
version cap like this, so that it works even when someone installs
without `--locked`.
obrok added a commit to Cardinal-Cryptography/ink-wrapper that referenced this issue May 22, 2023
proc-macro2 breaks genco (see
udoprog/genco#39). For now, we can introduce a
version cap like this, so that it works even when someone installs
without `--locked`.
@udoprog
Copy link
Owner

udoprog commented May 22, 2023

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

@obrok
Copy link
Author

obrok commented May 23, 2023

Not a problem and thanks for the quick responses!

@kklas
Copy link

kklas commented May 30, 2023

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

@udoprog
Copy link
Owner

udoprog commented May 30, 2023

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
Copy link
Owner

udoprog commented May 30, 2023

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
Copy link

kklas commented May 30, 2023

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
Copy link
Owner

udoprog commented May 30, 2023

Ah, definitely not, it's syn2 which has bumped the dependency: dtolnay/syn@ee29399

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
Copy link

kklas commented May 30, 2023

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
Copy link
Owner

udoprog commented May 30, 2023

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

kjuulh added a commit to gerhard/dagger that referenced this issue Jun 30, 2023
Signed-off-by: kjuulh <contact@kjuulh.io>
kjuulh added a commit to dagger/dagger that referenced this issue Jul 1, 2023
* Fix Rust SDK errors

They started happening right after
#5372 got merged, even though this
commit didn't change anything in the Rust SDK.

Updating all dependencies to latest via `cargo update` & re-running
the Rust SDK codegen (which seems to create incorrect comments) fixed
the issue for both lint & test.

Signed-off-by: Gerhard Lazu <gerhard@dagger.io>

* feat: temporary fix waiting for: udoprog/genco/issues/39

Signed-off-by: kjuulh <contact@kjuulh.io>

* feat: with stable version

Signed-off-by: kjuulh <contact@kjuulh.io>

* feat: with common variables

Signed-off-by: kjuulh <contact@kjuulh.io>

* feat: with rustfmt in base image

Signed-off-by: kjuulh <contact@kjuulh.io>

---------

Signed-off-by: Gerhard Lazu <gerhard@dagger.io>
Signed-off-by: kjuulh <contact@kjuulh.io>
Co-authored-by: kjuulh <contact@kjuulh.io>
@udoprog
Copy link
Owner

udoprog commented Aug 21, 2023

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
Copy link
Author

obrok commented Sep 18, 2023

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
Copy link
Author

obrok commented Sep 27, 2023

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
Copy link
Owner

udoprog commented Sep 27, 2023

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.

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

No branches or pull requests

3 participants