Skip to content
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

Use BitVector for global sets of AttrId #52799

Merged
merged 2 commits into from
Aug 1, 2018

Conversation

Mark-Simulacrum
Copy link
Member

No description provided.

@rust-highfive
Copy link
Collaborator

r? @michaelwoerister

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 28, 2018
@rust-highfive

This comment has been minimized.

@@ -47,7 +47,11 @@ impl<C: Idx> BitVector<C> {
#[inline]
pub fn contains(&self, bit: C) -> bool {
let (word, mask) = word_mask(bit);
(self.data[word] & mask) != 0
if let Some(word) = self.data.get(word) {
Copy link
Contributor

@Pratyush Pratyush Jul 28, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tiny Nit (feel free to ignore): this could be written as self.data.get(word).map_or(false, |word| (word & mask) != 0)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

map_or etc usually makes me more confused then seeing code like this so I prefer this in general when not in a conditional or other space-constrained place.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's completely reasonable =)

slot.resize(idx + 1, 0);
}
slot[idx] |= 1 << shift;
slot.insert(attr.id);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you mind changing this (and the other cases below) to just globals.used_attrs.lock().insert(attr.id);? The slot binding does not really add much to the codes readability.

@michaelwoerister
Copy link
Member

Thanks for the PR, @Mark-Simulacrum!

The changes to attribute tracking look good to me. However, the PR changes the interface of BitVector. It's a semantic difference whether the program panics when I try to access something outside the given capacity or whether it returns false. I'd prefer to keep the existing interface. It's slightly more efficient and might allow to catch some logic errors via index out of bounds failures.

You could add a new type BitSet or DenseBitSet with the given semantics. Internally it could just use a BitVector to ease the implementation.

@Mark-Simulacrum
Copy link
Member Author

I've split out the BitVector changes into a separate commit; instead of having BitSet I went with BitArray since it's non-growable. Happy to bikeshed on naming.

@rust-highfive
Copy link
Collaborator

The job x86_64-gnu-llvm-5.0 of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
[00:06:06] 
[00:06:06] error: Could not compile `rustc_data_structures`.
[00:06:06] 
[00:06:06] Caused by:
[00:06:06]   process didn't exit successfully: `/checkout/obj/build/bootstrap/debug/rustc --crate-name rustc_data_structures librustc_data_structures/lib.rs --color always --error-format json --crate-type dylib --emit=dep-info,link -C prefer-dynamic -C opt-level=2 -C metadata=3d8079c0b5c752f1 -C extra-filename=-3d8079c0b5c752f1 --out-dir /checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/deps --target x86_64-unknown-linux-gnu -L dependency=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/deps -L dependency=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/release/deps --extern cfg_if=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/deps/libcfg_if-f1848c30fab0ba2e.rlib --extern ena=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/deps/libena-52cd1efcc942bacf.rlib --extern log=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/deps/liblog-eaf85f66e467efb7.rlib --extern parking_lot=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/deps/libparking_lot-c2d75c4ffd469ddc.rlib --extern parking_lot_core=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/deps/libparking_lot_core-6ddf17f910242635.rlib --extern rustc_hash=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/deps/librustc_hash-50d2fcfa7f0ff0df.rlib --extern rustc_rayon=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/deps/librustc_rayon-c7c8f17fb8a8edba.rlib --extern rustc_rayon_core=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/deps/librustc_rayon_core-18ba5ff3e9db22d8.rlib --extern rustc_cratesio_shim=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/deps/librustc_cratesio_shim-ff1c9259a238aca6.so --extern serialize=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/deps/libserialize-a86c1222f09eecc5.so --extern serialize=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/deps/libserialize-a86c1222f09eecc5.rlib --extern stable_deref_trait=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/deps/libstable_deref_trait-fbfca448f722407d.rlib` (exit code: 101)
[00:06:06] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "build" "--target" "x86_64-unknown-linux-gnu" "-j" "4" "--release" "--locked" "--color" "always" "--features" " jemalloc" "--manifest-path" "/checkout/src/rustc/Cargo.toml" "--message-format" "json"
[00:06:06] expected success, got: exit code: 101
[00:06:06] thread 'main' panicked at 'cargo must succeed', bootstrap/compile.rs:1119:9
[00:06:06] travis_fold:end:stage0-rustc

[00:06:06] travis_time:end:stage0-rustc:start=1532964230585699432,finish=1532964278296806587,duration=47711107155


[00:06:06] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap build
[00:06:06] Build completed unsuccessfully in 0:01:49
[00:06:06] make: *** [all] Error 1
[00:06:06] Makefile:28: recipe for target 'all' failed

The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:1aefeb64
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)
---
travis_time:end:0189a240:start=1532964278836569574,finish=1532964278843143740,duration=6574166
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:06c9bc68
$ ln -s . checkout && for CORE in obj/cores/core.*; do EXE=$(echo $CORE | sed 's|obj/cores/core\.[0-9]*\.!checkout!\(.*\)|\1|;y|!|/|'); if [ -f "$EXE" ]; then printf travis_fold":start:crashlog\n\033[31;1m%s\033[0m\n" "$CORE"; gdb -q -c "$CORE" "$EXE" -iex 'set auto-load off' -iex 'dir src/' -iex 'set sysroot .' -ex bt -ex q; echo travis_fold":"end:crashlog; fi; done || true
travis_fold:end:after_failure.4
travis_fold:start:after_failure.5
travis_time:start:08e5c3fe
travis_time:start:08e5c3fe
$ cat ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers || true
cat: ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers: No such file or directory
travis_fold:end:after_failure.5
travis_fold:start:after_failure.6
travis_time:start:1d789fca

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@rust-highfive
Copy link
Collaborator

The job x86_64-gnu-llvm-5.0 of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
[00:18:06] note: Run with `RUST_BACKTRACE=1` for a backtrace.
[00:18:06] 
[00:18:06] error: internal compiler error: unexpected panic
[00:18:06] 
[00:18:06] note: the compiler unexpectedly panicked. this is a bug.
[00:18:06] 
[00:18:06] note: we would appreciate a bug report: https://github.com/rust-lang/rust/blob/master/CONTRIBUTING.md#bug-reports
[00:18:06] note: rustc 1.29.0-dev running on x86_64-unknown-linux-gnu
[00:18:06] 
[00:18:06] 
[00:18:06] note: compiler flags: -Z force-unstable-if-unmarked -C opt-level=2 -C prefer-dynamic -C debug-assertions=y -C link-args=-Wl,-rpath,$ORIGIN/../lib --crate-type lib
[00:18:06] 
[00:18:06] note: some of the compiler flags provided by cargo are hidden
[00:18:06] error: Could not compile `core`.
[00:18:06] 
[00:18:06] Caused by:
[00:18:06]   process didn't exit successfully: `/checkout/obj/build/bootstrap/debug/rustc --crate-name core libcore/lib.rs --color always --error-format json --crate-type lib --emit=dep-info,link -C opt-level=2 -C metadata=1cbcabaa1ea822b5 -C extra-filename=-1cbcabaa1ea822b5 --out-dir /checkout/obj/build/x86_64-unknown-linux-gnu/stage1-std/x86_64-unknown-linux-gnu/release/deps --target x86_64-unknown-linux-gnu -L dependency=/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-std/x86_64-unknown-linux-gnu/release/deps -L dependency=/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-std/release/deps` (exit code: 101)
---
156632 ./.git/modules/src
149120 ./src/llvm-emscripten/test
145460 ./obj/build/bootstrap/debug/incremental
130592 ./obj/build/bootstrap/debug/incremental/bootstrap-c7ee2tfsizs
130588 ./obj/build/bootstrap/debug/incremental/bootstrap-c7ee2tfsizs/s-f3e2bmxbw9-16dv7tg-3t5kexjst7huj
97532 ./obj/build/x86_64-unknown-linux-gnu/stage0/lib/rustlib/x86_64-unknown-linux-gnu/codegen-backends
89108 ./obj/build/x86_64-unknown-linux-gnu/stage1
89084 ./obj/build/x86_64-unknown-linux-gnu/stage1/lib
77076 ./.git/modules/src/tools
---
travis_time:end:10bb1b10:start=1532965780662148311,finish=1532965780668175720,duration=6027409
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:23132135
$ ln -s . checkout && for CORE in obj/cores/core.*; do EXE=$(echo $CORE | sed 's|obj/cores/core\.[0-9]*\.!checkout!\(.*\)|\1|;y|!|/|'); if [ -f "$EXE" ]; then printf travis_fold":start:crashlog\n\033[31;1m%s\033[0m\n" "$CORE"; gdb -q -c "$CORE" "$EXE" -iex 'set auto-load off' -iex 'dir src/' -iex 'set sysroot .' -ex bt -ex q; echo travis_fold":"end:crashlog; fi; done || true
travis_fold:end:after_failure.4
travis_fold:start:after_failure.5
travis_time:start:03a17c88
travis_time:start:03a17c88
$ cat ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers || true
cat: ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers: No such file or directory
travis_fold:end:after_failure.5
travis_fold:start:after_failure.6
travis_time:start:024aba71
$ dmesg | grep -i kill

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@rust-highfive
Copy link
Collaborator

The job x86_64-gnu-llvm-5.0 of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
[00:21:58] note: Run with `RUST_BACKTRACE=1` for a backtrace.
[00:21:58] 
[00:21:58] error: internal compiler error: unexpected panic
[00:21:58] 
[00:21:58] note: the compiler unexpectedly panicked. this is a bug.
[00:21:58] 
[00:21:58] note: we would appreciate a bug report: https://github.com/rust-lang/rust/blob/master/CONTRIBUTING.md#bug-reports
[00:21:58] note: rustc 1.29.0-dev running on x86_64-unknown-linux-gnu
[00:21:58] 
[00:21:58] 
[00:21:58] note: compiler flags: -Z force-unstable-if-unmarked -C opt-level=2 -C prefer-dynamic -C debug-assertions=y -C link-args=-Wl,-rpath,$ORIGIN/../lib --crate-type lib
[00:21:58] 
[00:21:58] note: some of the compiler flags provided by cargo are hidden
[00:21:58] error: Could not compile `core`.
[00:21:58] 
[00:21:58] Caused by:
[00:21:58]   process didn't exit successfully: `/checkout/obj/build/bootstrap/debug/rustc --crate-name core libcore/lib.rs --color always --error-format json --crate-type lib --emit=dep-info,link -C opt-level=2 -C metadata=1cbcabaa1ea822b5 -C extra-filename=-1cbcabaa1ea822b5 --out-dir /checkout/obj/build/x86_64-unknown-linux-gnu/stage1-std/x86_64-unknown-linux-gnu/release/deps --target x86_64-unknown-linux-gnu -L dependency=/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-std/x86_64-unknown-linux-gnu/release/deps -L dependency=/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-std/release/deps` (exit code: 101)
[00:21:58]   process didn't exit successfully: `/checkout/obj/build/bootstrap/debug/rustc --crate-name core libcore/lib.rs --color always --error-format json --crate-type lib --emit=dep-info,link -C opt-level=2 -C metadata=1cbcabaa1ea822b5 -C extra-filename=-1cbcabaa1ea822b5 --out-dir /checkout/obj/build/x86_64-unknown-linux-gnu/stage1-std/x86_64-unknown-linux-gnu/release/deps --target x86_64-unknown-linux-gnu -L dependency=/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-std/x86_64-unknown-linux-gnu/release/deps -L dependency=/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-std/release/deps` (exit code: 101)
[00:21:58] warning: build failed, waiting for other jobs to finish...
[00:22:03] error: build failed
[00:22:03] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "build" "--target" "x86_64-unknown-linux-gnu" "-j" "4" "--release" "--locked" "--color" "always" "--features" "panic-unwind jemalloc backtrace" "--manifest-path" "/checkout/src/libstd/Cargo.toml" "--message-format" "json"
[00:22:03] expected success, got: exit code: 101
[00:22:03] thread 'main' panicked at 'cargo must succeed', bootstrap/compile.rs:1119:9
[00:22:03] travis_fold:end:stage1-std

[00:22:03] travis_time:end:stage1-std:start=1532967509166067666,finish=1532967515831903293,duration=6665835627


[00:22:03] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap build
[00:22:03] Build completed unsuccessfully in 0:17:58
[00:22:03] make: *** [all] Error 1
[00:22:03] Makefile:28: recipe for target 'all' failed

The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:36005a98
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)
---
travis_time:end:2c76a2d2:start=1532967516398062882,finish=1532967516406257617,duration=8194735
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:267e7592
$ ln -s . checkout && for CORE in obj/cores/core.*; do EXE=$(echo $CORE | sed 's|obj/cores/core\.[0-9]*\.!checkout!\(.*\)|\1|;y|!|/|'); if [ -f "$EXE" ]; then printf travis_fold":start:crashlog\n\033[31;1m%s\033[0m\n" "$CORE"; gdb -q -c "$CORE" "$EXE" -iex 'set auto-load off' -iex 'dir src/' -iex 'set sysroot .' -ex bt -ex q; echo travis_fold":"end:crashlog; fi; done || true
travis_fold:end:after_failure.4
travis_fold:start:after_failure.5
travis_time:start:013051c1
travis_time:start:013051c1
$ cat ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers || true
cat: ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers: No such file or directory
travis_fold:end:after_failure.5
travis_fold:start:after_failure.6
travis_time:start:242581ce
$ dmesg | grep -i kill

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@Mark-Simulacrum
Copy link
Member Author

r? @michaelwoerister

@michaelwoerister
Copy link
Member

Thanks, Mark!

@bors r+

@bors
Copy link
Contributor

bors commented Jul 31, 2018

📌 Commit 6185bcfa8b574b78a84f43363427957ee35a0743 has been approved by michaelwoerister

@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 Jul 31, 2018
@Mark-Simulacrum
Copy link
Member Author

@bors r-

Merge conflicts

@bors bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 1, 2018
bors added a commit that referenced this pull request Aug 1, 2018
Rollup of 31 pull requests

Successful merges:

 - #52332 (dead-code lint: say "constructed" for structs)
 - #52340 (Document From trait implementations for OsStr, OsString, CString, and CStr)
 - #52628 (Cleanup some rustdoc code)
 - #52732 (Remove unstable and deprecated APIs)
 - #52745 (Update clippy to latest master)
 - #52756 (rustc: Disallow machine applicability in foreign macros)
 - #52771 (Clarify thread::park semantics)
 - #52810 ([NLL] Don't make "fake" match variables mutable)
 - #52821 (pretty print for std::collections::vecdeque)
 - #52822 (Fix From<LocalWaker>)
 - #52824 (Fix -Wpessimizing-move warnings in rustllvm/PassWrapper)
 - #52831 (remove references to AUTHORS.txt file)
 - #52835 (Fix Alias intra doc ICE)
 - #52842 (update comment)
 - #52846 (Add timeout to use of `curl` in bootstrap.py.)
 - #52851 (Make the tool_lints actually usable)
 - #52853 (Improve bootstrap help on stages)
 - #52859 (Use Vec::extend in SmallVec::extend when applicable)
 - #52861 (Add targets for HermitCore (https://hermitcore.org) to the Rust compiler and port libstd to it.)
 - #52867 (releases.md: fix 2 typos)
 - #52870 (Implement Unpin for FutureObj and LocalFutureObj)
 - #52876 (run-pass/const-endianness: negate before to_le())
 - #52878 (Fix wrong issue number in the test name)
 - #52883 (Include lifetime in mutability suggestion in NLL messages)
 - #52904 (NLL: sort diagnostics by span)
 - #52905 (Fix a typo in unsize.rs)
 - #52907 (NLL: On "cannot move out of type" error, print original before rewrite)
 - #52908 (Use SetLenOnDrop in Vec::truncate())
 - #52914 (Only run the sparc-abi test on sparc)
 - #52918 (Backport 1.27.2 release notes)
 - #52929 (Update compatibility note for 1.28.0 to be correct)

Failed merges:

 - #52758 (Cleanup for librustc::session)
 - #52799 (Use BitVector for global sets of AttrId)

r? @ghost
@Mark-Simulacrum
Copy link
Member Author

@bors r=michaelwoerister

@bors
Copy link
Contributor

bors commented Aug 1, 2018

📌 Commit 9bc4fbb has been approved by michaelwoerister

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 1, 2018
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this pull request Aug 1, 2018
…michaelwoerister

Use BitVector for global sets of AttrId
pietroalbini added a commit to pietroalbini/rust that referenced this pull request Aug 1, 2018
…michaelwoerister

Use BitVector for global sets of AttrId
bors added a commit that referenced this pull request Aug 1, 2018
Rollup of 15 pull requests

Successful merges:

 - #52793 (Add test for NLL: unexpected "free region `` does not outlive" error )
 - #52799 (Use BitVector for global sets of AttrId)
 - #52809 (Add test for unexpected region for local data ReStatic)
 - #52834 ([NLL] Allow conflicting borrows of promoted length zero arrays)
 - #52835 (Fix Alias intra doc ICE)
 - #52854 (fix memrchr in miri)
 - #52899 (tests/ui: Add missing mips{64} ignores)
 - #52908 (Use SetLenOnDrop in Vec::truncate())
 - #52915 (Don't count MIR locals as borrowed after StorageDead when finding locals live across a yield terminator)
 - #52926 (rustc: Trim down the `rust_2018_idioms` lint group)
 - #52930 (rustc_resolve: record single-segment extern crate import resolutions.)
 - #52939 (Make io::Read::read_to_end consider io::Take::limit)
 - #52942 (Another SmallVec.extend optimization)
 - #52947 (1.27 actually added the `armv5te-unknown-linux-musleabi` target)
 - #52954 (async can begin expressions)

Failed merges:

r? @ghost
@bors bors merged commit 9bc4fbb into rust-lang:master Aug 1, 2018
@Mark-Simulacrum Mark-Simulacrum deleted the attr-id-bitvecs branch June 8, 2019 13:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants