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

brson · 2015-03-22T19:06:46Z

See #23616 (comment) for a current issue description. The original description follows below.

Pretty-expanded source that results in unstable code can't build. After it becomes illegal to not declare features this is going to cause a bunch of expanded source to not build.

brson · 2015-03-22T19:07:18Z

I'm going to be adding no-pretty-expanded to a bunch of tests.

steveklabnik · 2015-03-22T19:13:40Z

at least expanded is unstable, yeah?

alexcrichton · 2015-03-22T19:29:44Z

Unfortunately I think that this is just a fundamental limitation. We have a number of problems that make it such that expanded source cannot actually compile at all:

#[allow_internal_unstable] make it such that macros can use unstable features without declaration
#[no_std] which is injected is unstable
Hygiene (e.g. a macro's local variable foo is very different than the outer variable foo)

I would also be fine just assuming that tests cannot be pretty printed when expected unless there is an explicit opt-in directive.

brson · 2015-03-22T19:49:14Z

Perhaps I will switch compiletest to support pretty-expanded and add that to a bunch that are passing now. It does seem like it would be pretty annoying to maintain all the no-pretty-expanded flags.

nikomatsakis · 2015-04-08T08:28:34Z

can we just disable the pretty-printing tests for the beta branch or something like that?

nikomatsakis · 2015-04-08T08:35:48Z

(Does that make sense?)

steveklabnik · 2016-06-27T21:32:27Z

Triage: not aware of any specific issue here, though I haven't heard about anyone running into this in practice in the last year.

steveklabnik · 2018-09-24T17:08:52Z

Triage: lots of FIXMEs referring to this issue still remain.

jethrogb · 2019-04-19T19:59:15Z

Ran into this while looking at the compiletest framework and was thoroughly confused. As far as I can tell, this is the situation as of early 2019:

Certain parts of the Rust test suite can be run in “pretty” mode. This will test the pretty-printing output of rustc. One of the pretty-printing outputs of rustc is the expanded mode, which may result in code that can't subsequently be compiled, that is what this issue is about. Tests that were in 2015 known to work with expanded mode have the pretty-expanded attribute to test that mode. All pretty-expanded annotations are marked with “FIXME #23616”, but this is misleading: the tests that have the attribute don't need to be fixed, it's the tests that don't have the pretty-expanded/FIXME annotation that are blocked on this issue.

I think it was a mistake to replace no-pretty-expanded with pretty-expanded in #23598, because the test coverage of expanded pretty printing has basically stagnated. Since 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. Now, everyone has basically forgotten about this attribute.

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

brson mentioned this issue Mar 22, 2015

Make it an error to not declare used features #23598

Closed

steveklabnik added the A-pretty Area: Pretty printing (including `-Z unpretty`) label Mar 22, 2015

This was referenced Apr 8, 2015

Destabilise the internals of format_args! #22953

Closed

std: Destabilize the internals of panic! #24212

Merged

pnkfelix mentioned this issue Jun 7, 2015

expansion-based box exacerbates pretty-printing to unstableness #26067

Closed

durka mentioned this issue Apr 25, 2016

special-case #[derive(Copy, Clone)] with a shallow clone #31414

Merged

petrochenkov mentioned this issue Nov 15, 2016

pretty-rpass tests work only accidentally/fragilely #15189

Closed

Mark-Simulacrum added the C-bug Category: This is a bug. label Jul 22, 2017

Mark-Simulacrum mentioned this issue Jun 13, 2020

Remove vestigial CI job msvc-aux. #73296

Merged

jyn514 added the A-testsuite Area: The testsuite used to check the correctness of rustc label Apr 3, 2023

jyn514 changed the title ~~Pretty-expanded source that results in unstable code can't build.~~ There are very few tests for -Z unpretty expansion Apr 3, 2023

jieyouxu added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Nov 25, 2024

jieyouxu mentioned this issue 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

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

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

brson commented Mar 22, 2015 •

edited by jyn514

Loading

brson commented Mar 22, 2015

steveklabnik commented Mar 22, 2015

alexcrichton commented Mar 22, 2015

brson commented Mar 22, 2015

nikomatsakis commented Apr 8, 2015

nikomatsakis commented Apr 8, 2015

steveklabnik commented Jun 27, 2016

steveklabnik commented Sep 24, 2018

jethrogb commented Apr 19, 2019 •

edited

Loading

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

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

Comments

brson commented Mar 22, 2015 • edited by jyn514 Loading

brson commented Mar 22, 2015

steveklabnik commented Mar 22, 2015

alexcrichton commented Mar 22, 2015

brson commented Mar 22, 2015

nikomatsakis commented Apr 8, 2015

nikomatsakis commented Apr 8, 2015

steveklabnik commented Jun 27, 2016

steveklabnik commented Sep 24, 2018

jethrogb commented Apr 19, 2019 • edited Loading

brson commented Mar 22, 2015 •

edited by jyn514

Loading

jethrogb commented Apr 19, 2019 •

edited

Loading