tests: fix: parallel frontend test failures: different alloc ids#156716
tests: fix: parallel frontend test failures: different alloc ids#156716heinwol wants to merge 5 commits into
Conversation
This comment has been minimized.
This comment has been minimized.
f681281 to
5abbf45
Compare
|
|
|
Reminder, once the PR becomes ready for a review, use |
5abbf45 to
bb760c1
Compare
|
@rustbot ready |
|
r? @RalfJung My assumptions were that
So it's better to just normalize the IDs away and enable these tests during parallel frontend testing, instead of ignoring them or waiting for some potential fix in the compiler. |
|
|
This comment has been minimized.
This comment has been minimized.
Spurious failure. |
|
A while back @oli-obk wrote some code somewhere to renumber AllocId while ensuring different ID remain differentiable. Those are the IDs that you are currently seeing in the test output ( On the one hand it is a bit unfortunate if we have no way of testing whether we actually print consistent IDs. On the other hand these IDs are mostly meaningless and using them in diagnostics isn't great to begin with. |
|
Seems fine as a short term solution. Having it as a global filter does seem better than repeating it. There's basically two orthogonal things that we should probably do:
|
@heinwol could you try this today? |
Unfortunately not. There are obvious ones like the ones pointing to statics or fns, but we don't have spans in |
This comment has been minimized.
This comment has been minimized.
|
Yeah getting span info is a Project on its own. It'd be nice to have but shouldn't block the parallel frontend IMO. Personally I could live with normalizing all AllocId away. |
This comment has been minimized.
This comment has been minimized.
054a0b1 to
d5a7959
Compare
This comment has been minimized.
This comment has been minimized.
d5a7959 to
0228182
Compare
This comment has been minimized.
This comment has been minimized.
0228182 to
fb5a0e4
Compare
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
fb5a0e4 to
877a131
Compare
in `tests/ui/consts/const-eval/raw-bytes.rs` as we now handle this in our modified regex in `runtest`
Specifically, all `//@ ignore-parallel-frontend different alloc ids`
across other tests because they fail with our normalization strategy: `ALLOC0` -> `ALLOC$ID`
|
@rustbot ready |
|
Some changes occurred in src/tools/compiletest cc @jieyouxu |
|
Requested reviewer is already assigned to this pull request. Please choose another assignee. |
|
The job Click to see the possible cause of the failure (guessed by this bot) |
|
A lot of failures in |
| // The alloc-id appears in pretty-printed allocations. | ||
| normalized = static_regex!( | ||
| r"╾─*a(lloc)?([0-9]+)(\+0x[0-9a-f]+)?(<imm>)?( \([0-9]+ ptr bytes\))?─*╼" | ||
| r"╾─*(?:a(?:lloc)?|A(:?LLOC)?)\d+(:?\+0x[[:alnum:]]+)?(:?<imm>)?(:? ?\(\d+ ptr bytes\))?─*╼" |
There was a problem hiding this comment.
Any reason you replaced 0-9a-f by :alnum:? That makes it less correct.
There was a problem hiding this comment.
Also there's ?:lloc and :?LLOC. I don't know either of these syntaxes, but the fact that they are different seems wrong.
View all comments
Removed the
//@ ignore-parallel-frontend different alloc idsdirective from all the ui tests applicable and replaced it with//@ normalize-stderrso that all alloc ids mentions are converted into a single placeholder. Thus the tests are passedPart of #154314
r? @petrochenkov