Skip to content

Conversation

@eggyal
Copy link
Contributor

@eggyal eggyal commented Jan 21, 2026

This method was broken by 258ace6, which changed self.normalized_pos to use relative offsets however this method continued to compare against an absolute offset.

Also adds a regression test for the issue that this method was originally introduced to fix.

Closes #149568
Fixes regression of #110885

r? cjgillot (as author of the breaking commit)

@rustbot rustbot added A-tidy Area: The tidy tool S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jan 21, 2026
@rust-log-analyzer

This comment has been minimized.

@eggyal
Copy link
Contributor Author

eggyal commented Jan 21, 2026

@rustbot author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 21, 2026
@rustbot
Copy link
Collaborator

rustbot commented Jan 21, 2026

Reminder, once the PR becomes ready for a review, use @rustbot ready.

@eggyal eggyal marked this pull request as draft January 21, 2026 13:44
@eggyal eggyal force-pushed the normalized-byte-pos branch 4 times, most recently from 14cd5b1 to d4d3476 Compare January 21, 2026 16:59
@eggyal eggyal marked this pull request as ready for review January 21, 2026 17:00
@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 21, 2026
@eggyal
Copy link
Contributor Author

eggyal commented Jan 21, 2026

The test was failing on aarch64 because it uses a different asm syntax/language, so my # prefixed comments were rejected. This has been fixed by replacing with // prefixed comments, which are valid in both the Intel and Arm asm syntaxes.

However, differences in the precise error output required manual normalization. I suspect we should instead enable this test just for Intel architectures? Otherwise there will no doubt be other syntax/normalization failures on other targets?

@rustbot ready

@rust-log-analyzer

This comment has been minimized.

This method was broken by 258ace6, which changed `self.normalized_pos`
to use relative offsets however this method continued to compare against
an absolute offset.

Also adds a regression test for the issue that this method was
originally introduced to fix.
@eggyal eggyal force-pushed the normalized-byte-pos branch from d4d3476 to 01290cc Compare January 21, 2026 18:42
@rustbot
Copy link
Collaborator

rustbot commented Jan 21, 2026

Some changes occurred in src/tools/compiletest

cc @jieyouxu

@rustbot rustbot added A-compiletest Area: The compiletest test runner A-testsuite Area: The testsuite used to check the correctness of rustc labels Jan 21, 2026
@eggyal
Copy link
Contributor Author

eggyal commented Jan 21, 2026

Okay, so it was then failing on gcc which again isn't a surprise. So I've reverted the test normalization and restricted it to x86_64 on llvm; however the needs-backends directive (introduced in #144125) wasn't being recognised—hence 7e39015 (which should probably be in a separate PR? but it's soooo minor....).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-compiletest Area: The compiletest test runner A-testsuite Area: The testsuite used to check the correctness of rustc A-tidy Area: The tidy tool S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) 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.

Method SourceFile::normalized_byte_pos is broken

4 participants