-
Notifications
You must be signed in to change notification settings - Fork 13.6k
resolve: Make disambiguators for underscore bindings module-local (take 2) #144272
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
r? @davidtwco rustbot has assigned @davidtwco. Use |
4310ac2
to
e94b3da
Compare
@bors r+ rollup |
resolve: Make disambiguators for underscore bindings module-local (take 2) The difference with rust-lang#144013 can be seen in the second commit. Now we just keep a separate disambiguator counter in every `Module`, instead of a global counter in `Resolver`. This will be ok for parallel import resolution because we'll need to lock the module anyway when updating `resolutions` and other fields in it. And for external modules the disabmiguator could be just passed as an argument to `define_extern`, without using any cells or locks, once rust-lang#143884 lands. Unblocks rust-lang#143884.
Rollup of 8 pull requests Successful merges: - #144094 (Ensure we codegen the main fn) - #144173 (Remove tidy checks for `tests/ui/issues/`) - #144218 (Use serde for target spec json deserialize) - #144221 (generate elf symbol version in raw-dylib) - #144234 (Fix broken TLS destructors on 32-bit win7) - #144256 (Don't ICE on non-TypeId metadata within TypeId) - #144272 (resolve: Make disambiguators for underscore bindings module-local (take 2)) - #144276 (Use less HIR in check_private_in_public.) r? `@ghost` `@rustbot` modify labels: rollup
resolve: Make disambiguators for underscore bindings module-local (take 2) The difference with rust-lang#144013 can be seen in the second commit. Now we just keep a separate disambiguator counter in every `Module`, instead of a global counter in `Resolver`. This will be ok for parallel import resolution because we'll need to lock the module anyway when updating `resolutions` and other fields in it. And for external modules the disabmiguator could be just passed as an argument to `define_extern`, without using any cells or locks, once rust-lang#143884 lands. Unblocks rust-lang#143884.
resolve: Make disambiguators for underscore bindings module-local (take 2) The difference with rust-lang#144013 can be seen in the second commit. Now we just keep a separate disambiguator counter in every `Module`, instead of a global counter in `Resolver`. This will be ok for parallel import resolution because we'll need to lock the module anyway when updating `resolutions` and other fields in it. And for external modules the disabmiguator could be just passed as an argument to `define_extern`, without using any cells or locks, once rust-lang#143884 lands. Unblocks rust-lang#143884.
@bors r- |
e94b3da
to
698fba9
Compare
I cannot reproduce this locally neither on Windows, nor on Linux. |
@bors try jobs=x86_64-apple-1 |
resolve: Make disambiguators for underscore bindings module-local (take 2) try-job: x86_64-apple-1
This comment has been minimized.
This comment has been minimized.
💔 Test failed (CI). Failed jobs:
|
I have a Mac M1. I can try and use Rosetta if you think that would help. |
This comment has been minimized.
This comment has been minimized.
The CI run on I'll remove the debugging commit and r+ this again with rollup=never. |
If you can reproduce the failure from #144325 (comment), it would be helpful, even if it's not related to this PR. |
If it's not related, it would be too much of a hassle. I will have to install all x86_64 Apple packages that are used by Rust. That will take too much space and time 😆. |
698fba9
to
4b55791
Compare
@bors r=oli-obk rollup=never |
☀️ Test successful - checks-actions |
What is this?This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.Comparing 3c30dbb (parent) -> fc5af18 (this PR) Test differencesNo test diffs found Test dashboardRun cargo run --manifest-path src/ci/citool/Cargo.toml -- \
test-dashboard fc5af1813307d25a84d633f21e2e53c9376eb547 --output-dir test-dashboard And then open Job duration changes
How to interpret the job duration changes?Job durations can vary a lot, based on the actual runner instance |
Finished benchmarking commit (fc5af18): comparison URL. Overall result: ❌ regressions - please read the text belowOur benchmarks found a performance regression caused by this PR. Next Steps:
@rustbot label: +perf-regression Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (secondary -1.1%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesThis benchmark run did not return any relevant results for this metric. Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 468.586s -> 468.258s (-0.07%) |
The difference with #144013 can be seen in the second commit.
Now we just keep a separate disambiguator counter in every
Module
, instead of a global counter inResolver
.This will be ok for parallel import resolution because we'll need to lock the module anyway when updating
resolutions
and other fields in it.And for external modules the disabmiguator could be just passed as an argument to
define_extern
, without using any cells or locks, once #143884 lands.Unblocks #143884.