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

fix: macro crate cannot be compiled alone #4130

Merged
merged 3 commits into from
Jun 11, 2024

Conversation

sunng87
Copy link
Member

@sunng87 sunng87 commented Jun 10, 2024

I hereby agree to the terms of the GreptimeDB CLA.

Refer to a related PR or issue link (optional)

What's changed and what's your intention?

This patch fixes an issue that I came across while working on other modules sit in common. Our macro module cannot compile by itself.

src/common/macro on  main [$] is 📦 v0.8.1 via 🦀 v1.79.0-nightly 
❯ cargo check
    Blocking waiting for file lock on build directory
    Checking syn v1.0.109
    Checking common-macro v0.8.1 (/home/sunng/greptime/greptimedb/src/common/macro)
error[E0432]: unresolved imports `syn::ItemFn`, `syn::Signature`
   --> src/common/macro/src/admin_fn.rs:19:57
    |
19  |     parse_macro_input, Attribute, AttributeArgs, Ident, ItemFn, Signature, Type, TypePath,
    |                                                         ^^^^^^  ^^^^^^^^^ no `Signature` in the root
    |                                                         |
    |                                                         no `ItemFn` in the root
    |
    = help: consider importing this struct instead:
            syn2::ItemFn
note: found an item that was configured out
   --> /home/sunng/.cargo/registry/src/index.crates.io-6f17d22bba15001f/syn-1.0.109/src/lib.rs:365:32
    |
365 |     ItemEnum, ItemExternCrate, ItemFn, ItemForeignMod, ItemImpl, ItemMacro, ItemMacro2, ItemMod,
    |                                ^^^^^^
    = note: the item is gated behind the `full` feature
    = help: consider importing this struct instead:
            syn2::Signature
note: found an item that was configured out
   --> /home/sunng/.cargo/registry/src/index.crates.io-6f17d22bba15001f/syn-1.0.109/src/lib.rs:367:5
    |
367 |     Signature, TraitItem, TraitItemConst, TraitItemMacro, TraitItemMethod, TraitItemType, UseGlob,
    |     ^^^^^^^^^
    = note: the item is gated behind the `full` feature

error[E0432]: unresolved import `syn::ItemStruct`
   --> src/common/macro/src/aggr_func.rs:19:43
    |
19  | use syn::{parse_macro_input, DeriveInput, ItemStruct};
    |                                           ^^^^^^^^^^ no `ItemStruct` in the root
    |
    = help: consider importing this struct instead:
            syn2::ItemStruct
note: found an item that was configured out
   --> /home/sunng/.cargo/registry/src/index.crates.io-6f17d22bba15001f/syn-1.0.109/src/lib.rs:366:17
    |
366 |     ItemStatic, ItemStruct, ItemTrait, ItemTraitAlias, ItemType, ItemUnion, ItemUse, Receiver,
    |                 ^^^^^^^^^^
    = note: the item is gated behind the `full` feature

error[E0432]: unresolved import `syn::ItemFn`
   --> src/common/macro/src/print_caller.rs:17:45
    |
17  | use syn::{parse_macro_input, AttributeArgs, ItemFn, Lit, Meta, NestedMeta};
    |                                             ^^^^^^ no `ItemFn` in the root
    |
    = help: consider importing this struct instead:
            syn2::ItemFn
note: found an item that was configured out
   --> /home/sunng/.cargo/registry/src/index.crates.io-6f17d22bba15001f/syn-1.0.109/src/lib.rs:365:32
    |
365 |     ItemEnum, ItemExternCrate, ItemFn, ItemForeignMod, ItemImpl, ItemMacro, ItemMacro2, ItemMod,
    |                                ^^^^^^
    = note: the item is gated behind the `full` feature

error[E0432]: unresolved imports `syn::ItemFn`, `syn::Signature`
   --> src/common/macro/src/range_fn.rs:19:57
    |
19  |     parse_macro_input, Attribute, AttributeArgs, Ident, ItemFn, Signature, Type, TypeReference,
    |                                                         ^^^^^^  ^^^^^^^^^ no `Signature` in the root
    |                                                         |
    |                                                         no `ItemFn` in the root
    |
    = help: consider importing this struct instead:
            syn2::ItemFn
note: found an item that was configured out
   --> /home/sunng/.cargo/registry/src/index.crates.io-6f17d22bba15001f/syn-1.0.109/src/lib.rs:365:32
    |
365 |     ItemEnum, ItemExternCrate, ItemFn, ItemForeignMod, ItemImpl, ItemMacro, ItemMacro2, ItemMod,
    |                                ^^^^^^
    = note: the item is gated behind the `full` feature
    = help: consider importing this struct instead:
            syn2::Signature
note: found an item that was configured out
   --> /home/sunng/.cargo/registry/src/index.crates.io-6f17d22bba15001f/syn-1.0.109/src/lib.rs:367:5
    |
367 |     Signature, TraitItem, TraitItemConst, TraitItemMacro, TraitItemMethod, TraitItemType, UseGlob,
    |     ^^^^^^^^^
    = note: the item is gated behind the `full` feature

error[E0432]: unresolved import `syn::FnArg`
   --> src/common/macro/src/utils.rs:21:11
    |
21  | use syn::{FnArg, Ident, Meta, MetaNameValue, NestedMeta, Type};
    |           ^^^^^ no `FnArg` in the root
    |
    = help: consider importing this enum instead:
            syn2::FnArg
note: found an item that was configured out
   --> /home/sunng/.cargo/registry/src/index.crates.io-6f17d22bba15001f/syn-1.0.109/src/lib.rs:363:5
    |
363 |     FnArg, ForeignItem, ForeignItemFn, ForeignItemMacro, ForeignItemStatic, ForeignItemType,
    |     ^^^^^
    = note: the item is gated behind the `full` feature

warning: unused import: `syn::spanned::Spanned`
  --> src/common/macro/src/aggr_func.rs:18:5
   |
18 | use syn::spanned::Spanned;
   |     ^^^^^^^^^^^^^^^^^^^^^
   |
   = note: `#[warn(unused_imports)]` on by default

warning: unused import: `ToTokens`
  --> src/common/macro/src/print_caller.rs:16:20
   |
16 | use quote::{quote, ToTokens};
   |                    ^^^^^^^^

For more information about this error, try `rustc --explain E0432`.
warning: `common-macro` (lib) generated 2 warnings
error: could not compile `common-macro` (lib) due to 5 previous errors; 2 warnings emitted

The macro module depends on both syn 1.0 and syn 2.0. I have no idea why it works but it turns out working when you compile the whole thing from root level.

This patch completely removed usage of syn 1.0 from macros, upgraded to syn::meta::parser for getting attributes. It also changes data type of those macro attribute value from string literal to ident, because it's actually used as ident.

Checklist

  • I have written the necessary rustdoc comments.
  • I have added the necessary unit tests and integration tests.
  • This PR requires documentation updates.

@sunng87 sunng87 requested review from waynexia and a team as code owners June 10, 2024 23:00
@github-actions github-actions bot added the docs-not-required This change does not impact docs. label Jun 10, 2024
@sunng87 sunng87 requested a review from tisonkun June 10, 2024 23:01
Copy link

codecov bot commented Jun 10, 2024

Codecov Report

Attention: Patch coverage is 87.50000% with 14 lines in your changes missing coverage. Please review.

Project coverage is 85.16%. Comparing base (9cae15b) to head (c249c7f).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4130      +/-   ##
==========================================
- Coverage   85.43%   85.16%   -0.28%     
==========================================
  Files         994      994              
  Lines      174340   174348       +8     
==========================================
- Hits       148955   148482     -473     
- Misses      25385    25866     +481     

src/common/macro/Cargo.toml Outdated Show resolved Hide resolved
@tisonkun tisonkun enabled auto-merge June 11, 2024 05:20
@tisonkun tisonkun added this pull request to the merge queue Jun 11, 2024
Merged via the queue into GreptimeTeam:main with commit 587e99d Jun 11, 2024
42 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs-not-required This change does not impact docs.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants