Uh oh!
There was an error while loading. Please reload this page.
Remove potential cfgs duplicates - #66959
Conversation
There was a problem hiding this comment.
I don't understand this algorithm and suspect it's not correct (dedup_by_key is dedup but it compares something other than the value itself, in your case it dedups consecutive values which are both true or both false in the result of your closure).
I suspect you meant retain instead of dedup_by_key. However, it's still quadratic.
If you don't care about the order, you can use v.sort(); v.dedup();.
If you care about the order, you can use something more like this:
letmut seen = FxHashSet::new();
v.retain(|x| seen.insert(x));Or since you start from a slice:
letmut seen = FxHashSet::new();
s.iter().filter(|x| seen.insert(x)).collect()None of the above suggestions is quadratic, and they're all simpler.
There was a problem hiding this comment.
I didn't want to change the order but if you're fine with it then so be it! (it'll make the code simpler so yeay! \o/)
There was a problem hiding this comment.
I didn't want to change the order
I provided one example that changes the order and two that don't. The last one is probably what you should use.
Note that I'm only using the FxHashSet to check if the value has been seen already, but the set isn't iterated afterward, so the original order is kept.
eddyb
commented
Dec 2, 2019
Not sure what you mean by "metadata". It's possible there are |
rust-highfive
commented
Dec 2, 2019
The job Click to expand the log.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 |
ollie27
commented
Dec 2, 2019
Here's a small test case where both #![feature(doc_cfg)]#[doc(cfg(feature = "sync"))]#[doc(cfg(feature = "sync"))]pubstructFoo;#[doc(cfg(feature = "sync"))]pubmod bar {#[doc(cfg(feature = "sync"))]pubstructBar;}The de-duplication needs to happen in the rust/src/librustdoc/clean/cfg.rs Lines 32 to 35 in 2da942f |
db227a4 to
e170516Compareeddyb
commented
Dec 3, 2019
r=me after removing |
rust-highfive
commented
Dec 3, 2019
The job Click to expand the log.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 |
c9026ee to
69e5c9bCompare69e5c9b to
a8ec620CompareGuillaumeGomez
commented
Dec 4, 2019
Ok, so after a few checks, it's not where we thought we should change but instead in the |
eddyb
commented
Dec 5, 2019
@bors r+ |
bors
commented
Dec 5, 2019
📌 Commit a8ec620 has been approved by |
Remove potential cfgs duplicates Fixesrust-lang#66921. Before going any further (the issue seems to be linked to metadata as far as I can tell). Do you think this is the good place to do it or should it be done before? r? @eddyb
Rollup of 11 pull requests Successful merges: - #66846 (Make try_mark_previous_green aware of cycles.) - #66959 (Remove potential cfgs duplicates) - #66988 (Fix angle bracket formatting when dumping MIR debug vars) - #66998 (Modified the testcases for VxWorks) - #67008 (rustdoc: Add test for fixed issue) - #67023 (SGX: Fix target linker used by bootstrap) - #67033 (Migrate to LLVM{Get,Set}ValueName2) - #67049 (Simplify {IoSlice, IoSliceMut}::advance examples and tests) - #67054 (codegen "unreachable" for invalid SetDiscriminant) - #67081 (Fix Query type docs) - #67085 (Remove boxed closures in address parser.) Failed merges: r? @ghost
…umeGomez rustdoc: Remove more `#[doc(cfg(..))]` duplicates This is a follow up to rust-lang#66959. r? @GuillaumeGomez
Fixes#66921.
Before going any further (the issue seems to be linked to metadata as far as I can tell). Do you think this is the good place to do it or should it be done before?
r? @eddyb