Remove a footgun-y feature / relic of the past from the compiletest DSL #136404
Conversation
|
Yes this is known, I'll double-check why we didn't remove this a bit later today |
Prolly cuz of the long time window in which we kept trying to detect directives in non- |
It's actually very simple (since the comment says |
|
r? jieyouxu |
There was a problem hiding this comment.
I added a temp commit to fix the test, then benchmarked locally on x86_64-unknown-linux-gnu1:
hyperfine --runs 5 --ignore-failure "./x test tests/ui --stage 1 --force-rerun" --show-output
Before removing this "optimization" (by reverting this PR):
Build completed successfully in 0:01:16
Time (mean ± σ): 67.854 s ± 0.822 s [User: 918.613 s, System: 309.796 s]
Range (min … max): 67.287 s … 69.307 s 5 runs
After removing this "optimization":
Build completed successfully in 0:01:14
Time (mean ± σ): 67.683 s ± 0.332 s [User: 909.790 s, System: 305.177 s]
Range (min … max): 67.196 s … 68.064 s 5 runs
Looks good to me as well. Thanks for removing this!
Footnotes
-
This benchmark is very very coarse, I was only looking for big regressions. This obviously doesn't test cold page cache, but I don't think that's super worth to optimize for, versus actual problems of ignoring
//@in tests. ↩
3e0242c to
b7860f0
Compare
b7860f0 to
f88f0a8
Compare
|
I had to update |
|
Some changes occurred in src/tools/compiletest cc @jieyouxu |
Fortunately (or unfortunately) FileCheck doesn't care about the comment prefix |
|
@bors r=Noratrieb,jieyouxu rollup |
Rollup of 8 pull requests Successful merges: - rust-lang#136356 (Docs for f16 and f128: correct a typo and add details) - rust-lang#136404 (Remove a footgun-y feature / relic of the past from the compiletest DSL) - rust-lang#136432 (LTA: Actually check where-clauses for well-formedness at the def site) - rust-lang#136438 (miri: improve error when offset_from preconditions are violated) - rust-lang#136441 ([`compiletest`-related cleanups 1/7] Cleanup `is_rustdoc` logic and remove a useless path join in rustdoc-json runtest logic) - rust-lang#136455 (Remove some `Clone` bounds and derives.) - rust-lang#136464 (Remove hook calling via `TyCtxtAt`.) - rust-lang#136467 (override default config profile on tarballs) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#136404 - fmease:rm-compiletest-relic-of-the-past, r=Noratrieb,jieyouxu Remove a footgun-y feature / relic of the past from the compiletest DSL The compiletest DSL still features a historical remnant from the time when its directives were merely prefixed with `//` instead of `//`@`` when unknown directive names weren't rejected since they could just as well be part of prose: As an "optimization", it stops looking for directives once it stumbles upon a line which starts with either `fn` or `mod`. This is super footgun-y as it obviously leads to any seeming compiletest directives below `fn` and `mod` items getting completely ignored. See rust-lang#136403 for a practical example. As well the assembly test updated in this PR. ~~Blocked on rust-lang#136403.~~ (merged)
The compiletest DSL still features a historical remnant from the time when its directives were merely prefixed with
//instead of//@when unknown directive names weren't rejected since they could just as well be part of prose:As an "optimization", it stops looking for directives once it stumbles upon a line which starts with either
fnormod. This is super footgun-y as it obviously leads to any seeming compiletest directives belowfnandmoditems getting completely ignored.See #136403 for a practical example. As well the assembly test updated in this PR.
Blocked on #136403.(merged)