Build of v0.2.20-516-gea2d104 fails with errors about `modifier`

[sourcefrog]

I tried to build a clean checkout of ea2d104 with

rustc 1.63.0-beta.7 (12c5b1ef3 2022-07-22)

and also with

rustc 1.62.1 (e092d0b6b 2022-07-16)

and they both produce many errors like this

error[E0659]: `modifier` is ambiguous
    --> tests/integration/main.rs:46:26
     |
46   |           $(value.$field = modifier!(@value $field $($value)?);)+
     |                            ^^^^^^^^ ambiguous name
     |
    ::: tests/integration/parsing.rs:1010:26
     |
1010 |           Component::Month(modifier!(Month {
     |  __________________________-
1011 | |             padding: modifier::Padding::Space,
1012 | |             repr: modifier::MonthRepr::Numerical,
1013 | |         })),
     | |__________- in this macro invocation
     |
     = note: ambiguous because of a conflict between a `macro_rules` name and a non-`macro_rules` name from another module
note: `modifier` could refer to the macro defined here
    --> tests/integration/main.rs:41:1
     |
41   | / macro_rules! modifier {
42   | |     ($name:ident {
43   | |         $($field:ident $(: $value:expr)?),+ $(,)?
44   | |     }) => {{
...    |
51   | |     (@value $field:ident $value:expr) => ($value);
52   | | }
     | |_^
note: `modifier` could also refer to the unresolved item imported here
    --> tests/integration/parsing.rs:4:32
     |
4    | use time::format_description::{modifier, Component, FormatItem};
     |                                ^^^^^^^^
     = help: use `self::modifier` to refer to this unresolved item unambiguously
     = note: this error originates in the macro `modifier` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0659]: `modifier` is ambiguous
    --> tests/integration/parsing.rs:1018:26
     |
1018 |         Component::Month(modifier!(Month {
     |                          ^^^^^^^^ ambiguous name
     |
     = note: ambiguous because of a conflict between a `macro_rules` name and a non-`macro_rules` name from another module
note: `modifier` could refer to the macro defined here
    --> tests/integration/main.rs:41:1
     |
41   | / macro_rules! modifier {
42   | |     ($name:ident {
43   | |         $($field:ident $(: $value:expr)?),+ $(,)?
44   | |     }) => {{
...    |
51   | |     (@value $field:ident $value:expr) => ($value);
52   | | }
     | |_^
note: `modifier` could also refer to the unresolved item imported here
    --> tests/integration/parsing.rs:4:32
     |
4    | use time::format_description::{modifier, Component, FormatItem};
     |                                ^^^^^^^^
     = help: use `self::modifier` to refer to this unresolved item unambiguously

error[E0659]: `modifier` is ambiguous
    --> tests/integration/main.rs:46:26
     |
46   |           $(value.$field = modifier!(@value $field $($value)?);)+
     |                            ^^^^^^^^ ambiguous name
     |
    ::: tests/integration/parsing.rs:1018:26
     |
1018 |           Component::Month(modifier!(Month {
     |  __________________________-
1019 | |             padding: modifier::Padding::None,
1020 | |             repr: modifier::MonthRepr::Short,
1021 | |             case_sensitive: true,
1022 | |         })),
     | |__________- in this macro invocation
     |
     = note: ambiguous because of a conflict between a `macro_rules` name and a non-`macro_rules` name from another module
note: `modifier` could refer to the macro defined here
    --> tests/integration/main.rs:41:1
     |
41   | / macro_rules! modifier {
42   | |     ($name:ident {
43   | |         $($field:ident $(: $value:expr)?),+ $(,)?
44   | |     }) => {{
...    |
51   | |     (@value $field:ident $value:expr) => ($value);
52   | | }
     | |_^

[sourcefrog]

Strangely v0.2.20 also fails

error: unreachable `pub` item
   --> src/lib.rs:474:26
    |
474 |     pub use crate::ext::{NumericalDuration as _, NumericalStdDuration as _};
    |     ---                  ^^^^^^^^^^^^^^^^^^^^^^
    |     |
    |     help: consider restricting its visibility: `pub(crate)`
    |
note: the lint level is defined here
   --> src/lib.rs:156:5
    |
156 |     unreachable_pub,
    |     ^^^^^^^^^^^^^^^
    = help: or consider exporting it for use by other crates

error: unreachable `pub` item
   --> src/lib.rs:474:50
    |
474 |     pub use crate::ext::{NumericalDuration as _, NumericalStdDuration as _};
    |     ---                                          ^^^^^^^^^^^^^^^^^^^^^^^^^
    |     |
    |     help: consider restricting its visibility: `pub(crate)`
    |
    = help: or consider exporting it for use by other crates

error: could not compile `time` due to 2 previous errors
warning: build failed, waiting for other jobs to finish...
error: could not compile `time` due to 2 previous errors

[jhpratt]

If you're running cargo test, it is required that you pass --all-features. But that doesn't much matter — time 0.2 was superceded by time 0.3 well over a year ago and is unsupported.

[sourcefrog]

This was the tip of the default branch...

…

On Fri, Aug 5, 2022, 19:33 Jacob Pratt ***@***.***> wrote: If you're running cargo test, it is required that you pass --all-features. But that doesn't much matter — time 0.2 was superceded by time 0.3 well over a year ago and is unsupported. — Reply to this email directly, view it on GitHub <#497 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACUR42EFQRXDMVGK5RDUGDVXXFIVANCNFSM55WIBVBA> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[jhpratt]

You said you are using v0.2.20. That was released on September 16, 2020. I'm not sure what you're referring to with gea2d104, as no such commit exists in this repository. Nor do I know what the 516 is referring to. You are not using the current HEAD. There aren't even 474 lines in src/lib.rs currently.

Please provide sufficient information for me to work with. Your Cargo.toml would be ideal.

[sourcefrog]

That is the output from git describe on the tip of master in this repo as of yesterday, ea2d104. It is definitely in the repo as you can confirm by clicking this link. I realize the g prefix is confusing but that's git for you.

Apparently it's saying v0.2.20 because you switched from annotated to unannotated git tags at that point. It's the same revision as v0.3.12.

Anyhow cargo t --all-features does get the tests to compile and pass.

I don't see this mentioned anywhere in the docs, and it seems atypical among Rust crates, and not a good first experience of trying to assess this crate IMO. However it seems like this feedback is not welcome so I'll stop.

[jhpratt]

It's not uncommon for features to be required for testing. I'll add an explicit check to ensure that is the case, which would provide better diagnostics for anyone trying to run the test suite.

[sourcefrog]

Personally I think I would just guard the tests by the same feature flags, both so that it works by default and so that I can test it with various subsets enabled.

[jhpratt]

Just pushed that up. It's not ideal because it's at the start of stderr and other errors still occur, but it's something. It's also visible in the main.rs file. This is for both tests and benchmarks.

Personally I think I would just guard the tests by the same feature flags, both so that it works by default and so that I can test it with various subsets enabled.

This used to be what I did, but it wasn't worth the effort to maintain. The full powerset is still validated to an extent with cargo check, so it's quite unlikely that anything would break.

[sourcefrog]

In case it's interesting, while trying cargo-mutants on other crates I found that syn has a really slick solution to giving a clean error message for this:

https://github.com/dtolnay/syn/blob/master/tests/features/macro/lib.rs

   Compiling unicode-normalization v0.1.21
   Compiling flate2 v1.0.24
   Compiling syn-test-suite v0.0.0 (/home/mbp/src/syn/tests/features)

NOTE:  use --release
  Syn's test suite has some tests that run on every source file
  and test case in the rust-lang/rust repo, which can be pretty
  slow in debug mode. Consider running cargo test with `--release`
  to speed things up.

ERROR:  use --all-features
  Syn's test suite normally only works with all-features enabled.
  Run again with `--all-features`, or run with `--features test`
  to bypass this check.

error: could not compile `syn-test-suite`
warning: build failed, waiting for other jobs to finish...

*** cargo result: Exited(101) in 7.953s

(I'm not saying you're obliged, I just found it interesting.)

[jhpratt]

That's fundamentally different because it's a proc macro, so compilation can be stopped before type checking.