Make it an error to not declare used features #23598

brson · 2015-03-22T00:37:27Z

Also, on the beta/stable channels, it is an error to write feature.

I had to add two hacks to rustdoc to make the doctests support crate attributes correctly, and to avoid injecting 'extern crate' items that trip the unstable feature error for doctests inside the std facade. The latter of these adds a crate-level #[doc(test(no_inject_crate))] attribute, which is pretty ugly.

This is very prone to bitrot and will be hard to land.

rust-highfive · 2015-03-22T00:37:32Z

r? @aturon

(rust_highfive has picked a reviewer for you, use r? to override)

brson · 2015-03-22T02:03:58Z

Oh, still doesn't quite pass.

brson · 2015-03-22T02:15:42Z

Well, I hit an ugly bug because of the hack I put in to make crate attributes work in doc tests. The rustdoc test runner wants to put 'extern crate' and 'main' functions into the source, and it needs to do that after any crate attributes. To avoid putting explicit main functions into every test that declares a feature I made the test runner try to figure out the parts of the example that are outer attributes, and make them crate attributes.

Unfortunately, this appears in the book:

#![doc(html_logo_url = "http://www.rust-lang.org/logos/rust-logo-128x128-blk-v2.png",
       html_favicon_url = "http://www.rust-lang.org/favicon.ico",
       html_root_url = "http://doc.rust-lang.org/")];

And rustdoc now thinks it should turn it into something like:

#![doc(html_logo_url = "http://www.rust-lang.org/logos/rust-logo-128x128-blk-v2.png",
fn main() {
       html_favicon_url = "http://www.rust-lang.org/favicon.ico",
       html_root_url = "http://doc.rust-lang.org/")];
}

brson · 2015-03-22T02:17:01Z

Maybe I could just have rustdoc treat only #![feature specially as a hack to get this done.

brson · 2015-03-22T19:08:02Z

I've run into another major problem: #23616. This isn't as close to ready as I thought.

brson · 2015-03-23T00:54:07Z

This has passed tests locally.

aturon · 2015-03-23T16:11:43Z

I had a couple minor nits but basically am OK trying to get this landed (due to the bitrot issues).

I do worry a bit about adding yet more ways that tests can pedantically fail, which can be very annoying...

brson · 2015-03-23T17:10:33Z

Rebased again.

brson · 2015-03-23T17:12:12Z

Doesn't pass on mac/win yet.

brson · 2015-03-23T19:09:49Z

@bors r=aturon p=1000

bors · 2015-03-23T19:09:49Z

📌 Commit 97c8444 has been approved by aturon

alexcrichton · 2015-03-23T21:05:38Z

@bors: force

alexcrichton · 2015-03-23T21:09:58Z

@bors: r=aturon 97c8444 p=1000

bors · 2015-03-23T21:09:59Z

🙀 97c84447b65164731087ea82685580cc81424412 is not a valid commit SHA. Please try again with 5d8367e.

alexcrichton · 2015-03-23T21:10:11Z

@bors: r=aturon 5d8367e1e15df88653450bdb649619061f9472a6 p=1000

alexcrichton · 2015-03-23T21:26:01Z

@bors: force

bors · 2015-03-23T21:26:06Z

⌛ Testing commit 5d8367e with merge e067340...

bors · 2015-03-23T21:38:32Z

💔 Test failed - auto-mac-64-nopt-t

…e attributes This makes it possible to write `#![feature(foo)]` in doc tests.

So that collections doctests don't automatically fail themselves by injecting `extern crate collections` when they are mostly using the std facade.

brson · 2015-03-23T21:40:56Z

@bors r=aturon p=1000

bors · 2015-03-23T21:40:56Z

📌 Commit 8c93a79 has been approved by aturon

alexcrichton · 2015-03-23T21:42:10Z

@bors: force

bors · 2015-03-23T21:42:14Z

⌛ Testing commit 8c93a79 with merge 56bdd12...

bors · 2015-03-23T22:43:38Z

💔 Test failed - auto-mac-64-opt

Now that features must be declared expanded source often does not compile. This adds 'pretty-expanded' to a bunch of test cases that still work.

brson · 2015-03-23T23:08:02Z

@bors r+ p=1000

bors · 2015-03-23T23:08:02Z

📌 Commit 66efe89 has been approved by brson

Conflicts: src/compiletest/compiletest.rs src/libcollections/lib.rs src/librustc_back/lib.rs src/libserialize/lib.rs src/libstd/lib.rs src/libtest/lib.rs src/test/run-make/rustdoc-default-impl/foo.rs src/test/run-pass/env-home-dir.rs

alexcrichton · 2015-03-24T00:12:31Z

I'm putting the rollup (#23654) ahead of this b/c it includes this inside of it. (just FYI)

bors · 2015-03-24T03:09:40Z

⌛ Testing commit 66efe89 with merge 7e441d4...

bors · 2015-03-24T04:10:36Z

💔 Test failed - auto-mac-64-opt

bors · 2015-03-24T21:01:05Z

☔ The latest upstream changes (presumably #23654) made this pull request unmergeable. Please resolve the merge conflicts.

alexcrichton · 2015-03-24T21:02:17Z

Rolled into #23654

Foreword ======== Let us begin the journey to rediscover what the `//@ pretty-expanded` directive does, brave traveller -- "My good friend, [..] when I wrote that passage, God and I knew what it meant. It is possible that God knows it still; but as for me, I have totally forgotten." -- Johann Paul Friedrich Richter, 1826 We must retrace the steps of those before us, for history shall guide us in the present and inform us of the future. The Past ======== Originally there was some effort to introduce more test coverage for `-Z unpretty=expanded` (in 2015 this was called `--pretty=expanded`). In [Make it an error to not declare used features rust-lang#23598][pr-23598], there was a flip from `//@ no-pretty-expanded` (opt-out of `-Z unpretty=expanded` test) to `//@ pretty-expanded` (opt-in to `-Z unpretty=expanded` test). This was needed because back then the dedicated `tests/pretty` ("pretty") test suite did not existed, and the pretty tests were grouped together under `run-pass` tests (I believe `ui` test suite didn't exist back then either). Unfortunately, in this process the replacement `//@ pretty-expanded` directives contained a `FIXME rust-lang#23616` linking to [There are very few tests for `-Z unpretty` expansion rust-lang#23616][issue-23616]. But this was arguably backwards and somewhat misleading, as noted in <rust-lang#23616 (comment)>: The attribute is off by default and things just work if you don't test it, people have not been adding the `pretty-expanded` annotation to new tests even if it would work. Which basically renders this useless. The Present =========== As of Nov 2024, we have a dedicated `pretty` test suite, and some time over the years the split between `run-pass` into `ui` and `pretty` test suites caused all of the `//@ pretty-expanded` in `ui` tests to do absolutely nothing -- the compiletest logic for `pretty-expanded` only triggered in the *pretty* test suite, but none of the pretty tests use it. Oops. The Future ========== Nobody remembers this, nobody uses this, it's misleading in ui tests. Let's get rid of this directive altogether. [pr-23598]: rust-lang#23598 [issue-23616]: rust-lang#23616

Cleanup: delete `//@ pretty-expanded` directive This PR removes the `//@ pretty-expanded` directive support in compiletest and removes its usage inside ui tests because it does not actually do anything, and its existence is itself misleading. This PR is split into two commits: 1. The first commit just drops `pretty-expanded` directive support in compiletest. 2. The second commit is created by `sd '//@ pretty-expanded.*\n' '' tests/ui/**/*.rs`[^1], reblessing, and slightly adjusting some leading whitespace in a few tests. We can tell this is fully removed because compiletest doesn't complain about unknown directive when running the `ui` test suite. cc rust-lang#23616 ### History Originally, there was some effort to introduce more test coverage for `-Z unpretty=expanded` (in 2015 this was called `--pretty=expanded`). In [Make it an error to not declare used features rust-lang#23598][pr-23598], there was a flip from `//@ no-pretty-expanded` (opt-out of `-Z unpretty=expanded` test) to `//@ pretty-expanded` (opt-in to `-Z unpretty=expanded` test). This was needed because back then the dedicated `tests/pretty` ("pretty") test suite did not existed, and the pretty tests were grouped together under `run-pass` tests (I believe the `ui` test suite didn't exist back then either). Unfortunately, in this process the replacement `//@ pretty-expanded` directives contained a `FIXME rust-lang#23616` linking to [There are very few tests for `-Z unpretty` expansion rust-lang#23616][issue-23616]. But this was arguably backwards and somewhat misleading, as noted in [rust-lang#23616](rust-lang#23616 (comment)): The attribute is off by default and things just work if you don't test it, people have not been adding the `pretty-expanded` annotation to new tests even if it would work. Which basically renders this useless. ### Current status As of Nov 2024, we have a dedicated `pretty` test suite, and some time over the years the split between `run-pass` into `ui` and `pretty` test suites caused all the `//@ pretty-expanded` in `ui` tests to do absolutely nothing: the compiletest logic for `pretty-expanded` only triggers in the *pretty* test suite, but none of the pretty tests use it. Oops. Nobody remembers this, nobody uses this, it's misleading in ui tests. Let's get rid of this directive altogether. [pr-23598]: rust-lang#23598 [issue-23616]: rust-lang#23616 ### Follow-ups - [x] Yeet this directive from rustc-dev-guide docs. rust-lang/rustc-dev-guide#2147 [^1]: https://github.com/chmln/sd r? compiler

Rollup merge of rust-lang#133470 - jieyouxu:ugly, r=compiler-errors Cleanup: delete `//@ pretty-expanded` directive This PR removes the `//@ pretty-expanded` directive support in compiletest and removes its usage inside ui tests because it does not actually do anything, and its existence is itself misleading. This PR is split into two commits: 1. The first commit just drops `pretty-expanded` directive support in compiletest. 2. The second commit is created by `sd '//@ pretty-expanded.*\n' '' tests/ui/**/*.rs`[^1], reblessing, and slightly adjusting some leading whitespace in a few tests. We can tell this is fully removed because compiletest doesn't complain about unknown directive when running the `ui` test suite. cc rust-lang#23616 ### History Originally, there was some effort to introduce more test coverage for `-Z unpretty=expanded` (in 2015 this was called `--pretty=expanded`). In [Make it an error to not declare used features rust-lang#23598][pr-23598], there was a flip from `//@ no-pretty-expanded` (opt-out of `-Z unpretty=expanded` test) to `//@ pretty-expanded` (opt-in to `-Z unpretty=expanded` test). This was needed because back then the dedicated `tests/pretty` ("pretty") test suite did not existed, and the pretty tests were grouped together under `run-pass` tests (I believe the `ui` test suite didn't exist back then either). Unfortunately, in this process the replacement `//@ pretty-expanded` directives contained a `FIXME rust-lang#23616` linking to [There are very few tests for `-Z unpretty` expansion rust-lang#23616][issue-23616]. But this was arguably backwards and somewhat misleading, as noted in [rust-lang#23616](rust-lang#23616 (comment)): The attribute is off by default and things just work if you don't test it, people have not been adding the `pretty-expanded` annotation to new tests even if it would work. Which basically renders this useless. ### Current status As of Nov 2024, we have a dedicated `pretty` test suite, and some time over the years the split between `run-pass` into `ui` and `pretty` test suites caused all the `//@ pretty-expanded` in `ui` tests to do absolutely nothing: the compiletest logic for `pretty-expanded` only triggers in the *pretty* test suite, but none of the pretty tests use it. Oops. Nobody remembers this, nobody uses this, it's misleading in ui tests. Let's get rid of this directive altogether. [pr-23598]: rust-lang#23598 [issue-23616]: rust-lang#23616 ### Follow-ups - [x] Yeet this directive from rustc-dev-guide docs. rust-lang/rustc-dev-guide#2147 [^1]: https://github.com/chmln/sd r? compiler

rust-highfive assigned aturon Mar 22, 2015

brson force-pushed the gate branch from 93b9e1a to e4feb8a Compare March 23, 2015 00:53

brson force-pushed the gate branch from e4feb8a to 5d8367e Compare March 23, 2015 17:09

brson added 2 commits March 23, 2015 14:40

rustdoc: interpret all leading feature attributes in examples as crat…

3d365f6

…e attributes This makes it possible to write `#![feature(foo)]` in doc tests.

rustdoc: Add #[doc(test(no_inject_crate))] attribute

7770ea7

So that collections doctests don't automatically fail themselves by injecting `extern crate collections` when they are mostly using the std facade.

brson force-pushed the gate branch from 5d8367e to 8c93a79 Compare March 23, 2015 21:40

brson added 3 commits March 23, 2015 16:07

Require feature attributes, and add them where necessary

3a21c4b

Add #![feature] attributes to doctests

8db4cae

rustdoc: Replace no-pretty-expanded with pretty-expanded

66efe89

Now that features must be declared expanded source often does not compile. This adds 'pretty-expanded' to a bunch of test cases that still work.

brson force-pushed the gate branch from 8c93a79 to 66efe89 Compare March 23, 2015 23:07

alexcrichton closed this Mar 24, 2015

jethrogb mentioned this pull request Apr 19, 2019

There are very few tests for -Z unpretty expansion #23616

Closed

jieyouxu mentioned this pull request Nov 25, 2024

Cleanup: delete //@ pretty-expanded directive #133470

Merged

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make it an error to not declare used features #23598

Make it an error to not declare used features #23598

brson commented Mar 22, 2015

rust-highfive commented Mar 22, 2015

brson commented Mar 22, 2015

brson commented Mar 22, 2015

brson commented Mar 22, 2015

brson commented Mar 22, 2015

brson commented Mar 23, 2015

aturon commented Mar 23, 2015

brson commented Mar 23, 2015

brson commented Mar 23, 2015

brson commented Mar 23, 2015

bors commented Mar 23, 2015

alexcrichton commented Mar 23, 2015

alexcrichton commented Mar 23, 2015

bors commented Mar 23, 2015

alexcrichton commented Mar 23, 2015

alexcrichton commented Mar 23, 2015

bors commented Mar 23, 2015

bors commented Mar 23, 2015

brson commented Mar 23, 2015

bors commented Mar 23, 2015

alexcrichton commented Mar 23, 2015

bors commented Mar 23, 2015

bors commented Mar 23, 2015

brson commented Mar 23, 2015

bors commented Mar 23, 2015

alexcrichton commented Mar 24, 2015

bors commented Mar 24, 2015

bors commented Mar 24, 2015

bors commented Mar 24, 2015

alexcrichton commented Mar 24, 2015

Make it an error to not declare used features #23598

Make it an error to not declare used features #23598

Conversation

brson commented Mar 22, 2015

rust-highfive commented Mar 22, 2015

brson commented Mar 22, 2015

brson commented Mar 22, 2015

brson commented Mar 22, 2015

brson commented Mar 22, 2015

brson commented Mar 23, 2015

aturon commented Mar 23, 2015

brson commented Mar 23, 2015

brson commented Mar 23, 2015

brson commented Mar 23, 2015

bors commented Mar 23, 2015

alexcrichton commented Mar 23, 2015

alexcrichton commented Mar 23, 2015

bors commented Mar 23, 2015

alexcrichton commented Mar 23, 2015

alexcrichton commented Mar 23, 2015

bors commented Mar 23, 2015

bors commented Mar 23, 2015

brson commented Mar 23, 2015

bors commented Mar 23, 2015

alexcrichton commented Mar 23, 2015

bors commented Mar 23, 2015

bors commented Mar 23, 2015

brson commented Mar 23, 2015

bors commented Mar 23, 2015

alexcrichton commented Mar 24, 2015

bors commented Mar 24, 2015

bors commented Mar 24, 2015

bors commented Mar 24, 2015

alexcrichton commented Mar 24, 2015