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

Doctests now run on non-exported macro_rules!, but can't actually test the macro #97030

Open
cuviper opened this issue May 13, 2022 · 7 comments
Labels
A-doctests Area: Documentation tests, run by rustdoc regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Milestone

Comments

@cuviper
Copy link
Member

cuviper commented May 13, 2022

Since #88038 was resolved with #96630, doctests now run on macro_rules! even if the macro is not exported.

This could be considered a breaking change -- it did break CI for rayon when this macro example for internal use suddenly became a real test and failed.

The problem is, I can't figure out how to make a working doctest for that, since it's not exported in the public API. What is the intended way to do this? And if there's not really a way to do it, then maybe this change should be reverted so there isn't any break.

I thought of #[cfg_attr(doctest, macro_export)], which does make even older Rust try to test it, but that still can't reach the macro because it's still using the non-doctest build of the crate.


PS: I ended up making unit tests for that rayon macro, which is probably better in that case, but that doesn't answer how non-exported macro tests should work.

@cuviper cuviper added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. A-doctests Area: Documentation tests, run by rustdoc labels May 13, 2022
@oli-obk oli-obk added the regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. label May 16, 2022
@rustbot rustbot added the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label May 16, 2022
@apiraino apiraino removed the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label May 18, 2022
@GuillaumeGomez
Copy link
Member

cc @m-ysk

@Nemo157
Copy link
Member

Nemo157 commented May 23, 2022

One reason to keep the change is that doctests on private functions also run, even though they similarly can't access the function they are documenting. For consistency it makes sense that all doctests just run, and it's up to the dev to either ```ignore or to use // comment syntax to make it not a doctest.

zombiepigdragon added a commit to zombiepigdragon/nix that referenced this issue May 31, 2022
Due to rust-lang/rust#97030, cargo test will fail to doctest macros
unless they are exported, breaking the examples for libc_bitflags! and
libc_enum!.

Adds `ignore` to the examples for these macros to stop tests from
failing.
bors bot added a commit to nix-rust/nix that referenced this issue May 31, 2022
1730: Ignore doctests for unexported macros r=asomers a=zombiepigdragon

Due to rust-lang/rust#97030, cargo test will fail to doctest macros unless they are exported, breaking the examples for `libc_bitflags!` and `libc_enum!`.

Adds `ignore` to the examples for these macros to stop tests from failing.

`cargo test` already fails on cargo 1.62.0-beta.2, and the above issue makes it seem unlikely that this will be changed on the Rust side. If rust-lang/rust#96630 *does* get reverted, this PR can be closed/unmerged, although the test wasn't running beforehand, and it might be worth making this explicit regardless.

Co-authored-by: Alex Rawson <ajzecrom@gmail.com>
@jyn514
Copy link
Member

jyn514 commented Jun 20, 2022

I don't think this is a bug. This is consistent with tests for other non-macro items, which also can't use themselves in the test. The test might be testing something other than the defined item, in which case it's still useful to run.

@jonhoo
Copy link
Contributor

jonhoo commented Jun 30, 2022

Carrying over from #98735, this should be marked as regression-from-stable-to-stable. If it is indeed intended breakage, it should be added to the compatibility notes for the 1.62.0 release so that those adopting the new release understand why things broke and how to fix it.

@steffahn
Copy link
Member

@rustbot label regression-from-stable-to-stable, -regression-from-stable-to-nightly

@rustbot rustbot added regression-from-stable-to-stable Performance or correctness regression from one stable version to another. I-prioritize Issue: Indicates that prioritization has been requested for this issue. and removed regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. labels Jun 30, 2022
@CAD97
Copy link
Contributor

CAD97 commented Jul 1, 2022

PR to add it to a rustdoc section: #98749

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jul 1, 2022
Add macro_rules! rustdoc change to 1.62 relnotes

rust-lang#96630 was tagged <kbd>relnotes</kbd> but didn't make it into the notes. Given this is a compatibility issue (rust-lang#97030, rust-lang#98735, rust-lang#98743), it probably *should* be retroactively added.
@apiraino apiraino removed the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Jul 1, 2022
jannic added a commit to rp-rs/rp-hal that referenced this issue Jul 7, 2022
While the code block fails to compile, it's still code and not random text. Marking it with `ignore` allows for proper syntax highlighting, for example.

According to rust-lang/rust#97030 (comment) this is the proper fix, so this closes #374
rtzoeller added a commit to rtzoeller/nix that referenced this issue Jul 7, 2022
Rust 1.62.0 now tries to build and run examples for non-exported macros,
as it has been doing with non-public functions. Ignore the tests, as
there is not currently a way to invoke the macros from these examples.

See rust-lang/rust#97030.
9names pushed a commit to rp-rs/rp-hal that referenced this issue Jul 8, 2022
While the code block fails to compile, it's still code and not random text. Marking it with `ignore` allows for proper syntax highlighting, for example.

According to rust-lang/rust#97030 (comment) this is the proper fix, so this closes #374
@decathorpe
Copy link

This also affected us in Fedora Linux. It broke builds of multiple crates in our CI (alga, nix 0.22, 0.23, and 0.24, sequoia-ipc, and sequoia-openpgp-mt). It probably affects more crates that we don't see in our CI, because we don't run the test suite for them.

decathorpe added a commit to decathorpe/alga that referenced this issue Jul 14, 2022
This fixes building doctests with Rust 1.62, which now actually runs
doctests for non-exported macros:

rust-lang/rust#97030
rtzoeller pushed a commit to rtzoeller/nix that referenced this issue Jul 17, 2022
Due to rust-lang/rust#97030, cargo test will fail to doctest macros
unless they are exported, breaking the examples for libc_bitflags! and
libc_enum!.

Adds `ignore` to the examples for these macros to stop tests from
failing.
rtzoeller pushed a commit to nix-rust/nix that referenced this issue Jul 17, 2022
Due to rust-lang/rust#97030, cargo test will fail to doctest macros
unless they are exported, breaking the examples for libc_bitflags! and
libc_enum!.

Adds `ignore` to the examples for these macros to stop tests from
failing.
rtzoeller pushed a commit to nix-rust/nix that referenced this issue Jul 17, 2022
Due to rust-lang/rust#97030, cargo test will fail to doctest macros
unless they are exported, breaking the examples for libc_bitflags! and
libc_enum!.

Adds `ignore` to the examples for these macros to stop tests from
failing.
ExplodingWaffle pushed a commit to ExplodingWaffle/rp-hal that referenced this issue Aug 14, 2022
While the code block fails to compile, it's still code and not random text. Marking it with `ignore` allows for proper syntax highlighting, for example.

According to rust-lang/rust#97030 (comment) this is the proper fix, so this closes rp-rs#374
jjant added a commit to smithy-lang/smithy-rs that referenced this issue Oct 11, 2022
Rust 1.62 introduced a breaking change where doctests for non-exported
macros are now run by default. These don't compile because the macro
can't be imported in it.
See rust-lang/rust#97030 for more info.
jjant added a commit to smithy-lang/smithy-rs that referenced this issue Oct 17, 2022
* Run clippy --fix for Rust 1.62.0 and format the resulting code

* Run clippy --fix on all targets

* Run clippy --fix with all features enabled

* Avoid extra allocation

* Use more idiomatic assert

* Ignore noisy lint

* Update pyo3 and pyo3-asyncio to 0.17.0

* Implement Eq on aws-smithy-checksums::Error

* Implement Eq on Protocol

* Replace conditionals with range-containment

* Implement Eq on types in aws-smithy-types

* Implement Eq on types in aws-smithy-http-server-python

* Implement Eq on types in aws-smithy-eventstream

* Implement Eq on types in aws-smithy-xml

* Implement Eq on aws-sigv4

* Update CI to use Rust 1.62.0

* Add Eq for generated types that implement PartialEq

* Allow clippy::needless_return in generated code

* Remove unnecessary reborrow in http_serde

* Remove unnecessary borrow in operation_deser

* Add CHANGELOG entries

* Revert "Add Eq for generated types that implement PartialEq"

This reverts commit 5169bd9.

* Update pyo3 and pyo3-asyncio in generated code for python server

* Allow clippy::derive_partial_eq_without_eq on structs and builders

* Run clippy on tools

* Fix accidental move in generated code

* Revert "Allow clippy::derive_partial_eq_without_eq on structs and builders"

This reverts commit 068c63c.

* Fix another accidental move in generated code

* Undo unwanted change to model

* Re-add reborrow in HttpBindingGenerator

* Fix clippy::format-push-string in changelogger

* Fix more uses of str.push_str(&format!(...))

* Remove unnecessary parenthesis

* Run ktlint

* Update aws/rust-runtime/aws-http/src/content_encoding.rs

Co-authored-by: John DiSanti <jdisanti@amazon.com>

* Update aws/rust-runtime/aws-http/src/content_encoding.rs

Co-authored-by: John DiSanti <jdisanti@amazon.com>

* Ignore doctest for non-exported macro

Rust 1.62 introduced a breaking change where doctests for non-exported
macros are now run by default. These don't compile because the macro
can't be imported in it.
See rust-lang/rust#97030 for more info.

* Run cargo fmt

* Use $crate instead of crate in macro

* Revert "Implement Eq on types in aws-smithy-types"

This reverts commit c45a6b5.

* Revert "Implement Eq on types in aws-smithy-eventstream"

This reverts commit 78f4b07.

* Revert "Implement Eq on types in aws-smithy-xml"

This reverts commit 590f01a.

* Revert "Implement Eq on aws-sigv4"

This reverts commit d78bb62.

* Revert "Implement Eq on types in aws-smithy-http-server-python"

This reverts commit f2cd901.

* Revert "Implement Eq on aws-smithy-checksums::Error"

This reverts commit 5da1704.

Co-authored-by: Julian Antonielli <julianantonielli@gmail.com>
Co-authored-by: John DiSanti <jdisanti@amazon.com>
aws-sdk-rust-ci pushed a commit to awslabs/aws-sdk-rust that referenced this issue Oct 26, 2022
* Run clippy --fix for Rust 1.62.0 and format the resulting code

* Run clippy --fix on all targets

* Run clippy --fix with all features enabled

* Avoid extra allocation

* Use more idiomatic assert

* Ignore noisy lint

* Update pyo3 and pyo3-asyncio to 0.17.0

* Implement Eq on aws-smithy-checksums::Error

* Implement Eq on Protocol

* Replace conditionals with range-containment

* Implement Eq on types in aws-smithy-types

* Implement Eq on types in aws-smithy-http-server-python

* Implement Eq on types in aws-smithy-eventstream

* Implement Eq on types in aws-smithy-xml

* Implement Eq on aws-sigv4

* Update CI to use Rust 1.62.0

* Add Eq for generated types that implement PartialEq

* Allow clippy::needless_return in generated code

* Remove unnecessary reborrow in http_serde

* Remove unnecessary borrow in operation_deser

* Add CHANGELOG entries

* Revert "Add Eq for generated types that implement PartialEq"

This reverts commit 5169bd95aa4dbbfc77c23bf7415e98ebc6361733.

* Update pyo3 and pyo3-asyncio in generated code for python server

* Allow clippy::derive_partial_eq_without_eq on structs and builders

* Run clippy on tools

* Fix accidental move in generated code

* Revert "Allow clippy::derive_partial_eq_without_eq on structs and builders"

This reverts commit 068c63ca2030879584e7b602bd0648abab19cabe.

* Fix another accidental move in generated code

* Undo unwanted change to model

* Re-add reborrow in HttpBindingGenerator

* Fix clippy::format-push-string in changelogger

* Fix more uses of str.push_str(&format!(...))

* Remove unnecessary parenthesis

* Run ktlint

* Update aws/rust-runtime/aws-http/src/content_encoding.rs

Co-authored-by: John DiSanti <jdisanti@amazon.com>

* Update aws/rust-runtime/aws-http/src/content_encoding.rs

Co-authored-by: John DiSanti <jdisanti@amazon.com>

* Ignore doctest for non-exported macro

Rust 1.62 introduced a breaking change where doctests for non-exported
macros are now run by default. These don't compile because the macro
can't be imported in it.
See rust-lang/rust#97030 for more info.

* Run cargo fmt

* Use $crate instead of crate in macro

* Revert "Implement Eq on types in aws-smithy-types"

This reverts commit c45a6b5a56d391923efce3c884291c330218982d.

* Revert "Implement Eq on types in aws-smithy-eventstream"

This reverts commit 78f4b07344d2cbb9d06b30ffd9bad16031fdd83b.

* Revert "Implement Eq on types in aws-smithy-xml"

This reverts commit 590f01af7326bde7de97cae97feeedf593b9239b.

* Revert "Implement Eq on aws-sigv4"

This reverts commit d78bb62124c4b3a24a35bdd655995de11252d17f.

* Revert "Implement Eq on types in aws-smithy-http-server-python"

This reverts commit f2cd9018844130d441820742dc1cf1c7abeaa38b.

* Revert "Implement Eq on aws-smithy-checksums::Error"

This reverts commit 5da170405e12d5077f67b8f8e41c2319138244d4.

Co-authored-by: Julian Antonielli <julianantonielli@gmail.com>
Co-authored-by: John DiSanti <jdisanti@amazon.com>
rtzoeller pushed a commit to rtzoeller/nix that referenced this issue Nov 29, 2022
Due to rust-lang/rust#97030, cargo test will fail to doctest macros
unless they are exported, breaking the examples for libc_bitflags! and
libc_enum!.

Adds `ignore` to the examples for these macros to stop tests from
failing.
rtzoeller pushed a commit to rtzoeller/nix that referenced this issue Dec 2, 2022
Due to rust-lang/rust#97030, cargo test will fail to doctest macros
unless they are exported, breaking the examples for libc_bitflags! and
libc_enum!.

Adds `ignore` to the examples for these macros to stop tests from
failing.
rtzoeller pushed a commit to rtzoeller/nix that referenced this issue Dec 2, 2022
Due to rust-lang/rust#97030, cargo test will fail to doctest macros
unless they are exported, breaking the examples for libc_bitflags! and
libc_enum!.

Adds `ignore` to the examples for these macros to stop tests from
failing.
rtzoeller pushed a commit to rtzoeller/nix that referenced this issue Dec 2, 2022
Due to rust-lang/rust#97030, cargo test will fail to doctest macros
unless they are exported, breaking the examples for libc_bitflags! and
libc_enum!.

Adds `ignore` to the examples for these macros to stop tests from
failing.
rtzoeller pushed a commit to rtzoeller/nix that referenced this issue Dec 2, 2022
Due to rust-lang/rust#97030, cargo test will fail to doctest macros
unless they are exported, breaking the examples for libc_bitflags! and
libc_enum!.

Adds `ignore` to the examples for these macros to stop tests from
failing.
rtzoeller pushed a commit to rtzoeller/nix that referenced this issue Dec 2, 2022
Due to rust-lang/rust#97030, cargo test will fail to doctest macros
unless they are exported, breaking the examples for libc_bitflags! and
libc_enum!.

Adds `ignore` to the examples for these macros to stop tests from
failing.
rtzoeller pushed a commit to rtzoeller/nix that referenced this issue Dec 2, 2022
Due to rust-lang/rust#97030, cargo test will fail to doctest macros
unless they are exported, breaking the examples for libc_bitflags! and
libc_enum!.

Adds `ignore` to the examples for these macros to stop tests from
failing.
rtzoeller pushed a commit to rtzoeller/nix that referenced this issue Dec 2, 2022
Due to rust-lang/rust#97030, cargo test will fail to doctest macros
unless they are exported, breaking the examples for libc_bitflags! and
libc_enum!.

Adds `ignore` to the examples for these macros to stop tests from
failing.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-doctests Area: Documentation tests, run by rustdoc regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
Status: No status
Development

No branches or pull requests