Skip to content

Fix malformed error annotations in a UI test #136403

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
Feb 3, 2025

Conversation

fmease
Copy link
Member

@fmease fmease commented Feb 1, 2025

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 allowed a malformed error annotation of the form //@[…]~^^^ to go undetected & unexercised (as it's placed below fn main() {).

Obviously a character other than @ would've mangled the error annotation, too (but it might've caught the reviewer's eye). I specifically found this file because I ran rg '^(fn|mod)[\s\S]*?//@' tests/ui --multiline -trust to check how footgun-y that "special feature" of compiletest is.

@rustbot
Copy link
Collaborator

rustbot commented Feb 1, 2025

r? @chenyukang

rustbot has assigned @chenyukang.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Feb 1, 2025
@oli-obk
Copy link
Contributor

oli-obk commented Feb 1, 2025

r? @oli-obk @bors r+ rollup

@bors
Copy link
Collaborator

bors commented Feb 1, 2025

📌 Commit 43b729d has been approved by oli-obk

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 1, 2025
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 2, 2025
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#136145 (Test validity of pattern types)
 - rust-lang#136339 (CompileTest: Add Directives to Ignore `arm-unknown-*` Targets)
 - rust-lang#136403 (Fix malformed error annotations in a UI test)
 - rust-lang#136414 (Shorten error message for callable with wrong return type)
 - rust-lang#136425 (Move `rustc_middle::infer::unify_key`)
 - rust-lang#136426 (Explain why we retroactively change a static initializer to have a different type)
 - rust-lang#136445 (Couple of cleanups to DiagCtxt and EarlyDiagCtxt)
 - rust-lang#136452 (Miri subtree update)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 2, 2025
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#136145 (Test validity of pattern types)
 - rust-lang#136339 (CompileTest: Add Directives to Ignore `arm-unknown-*` Targets)
 - rust-lang#136403 (Fix malformed error annotations in a UI test)
 - rust-lang#136414 (Shorten error message for callable with wrong return type)
 - rust-lang#136425 (Move `rustc_middle::infer::unify_key`)
 - rust-lang#136426 (Explain why we retroactively change a static initializer to have a different type)
 - rust-lang#136445 (Couple of cleanups to DiagCtxt and EarlyDiagCtxt)
 - rust-lang#136452 (Miri subtree update)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 5ab356f into rust-lang:master Feb 3, 2025
6 checks passed
@rustbot rustbot added this to the 1.86.0 milestone Feb 3, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Feb 3, 2025
Rollup merge of rust-lang#136403 - fmease:fix-a-ui-test, r=oli-obk

Fix malformed error annotations in a UI test

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 allowed a malformed error annotation of the form `//`@[…]~^^^`` to go undetected & unexercised (as it's placed below `fn main() {`).

Obviously a character other than ``@`` would've mangled the error annotation, too (but it might've caught the reviewer's eye). I specifically found this file because I ran `rg '^(fn|mod)[\s\S]*?//`@'` tests/ui --multiline -trust` to check how footgun-y that "special feature" of compiletest is.
@fmease fmease deleted the fix-a-ui-test branch February 3, 2025 02:04
jieyouxu added a commit to jieyouxu/rust that referenced this pull request Feb 3, 2025
…-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)
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Feb 3, 2025
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)
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants