Move collections from libextra into libcollections #12010

HeroesGrave · 2014-02-03T07:14:14Z

Part of #8784

Changes:

Everything labeled under collections in libextra has been moved into a new crate 'libcollection'.
Renamed container.rs to deque.rs, since it was no longer 'container traits for extra', just a deque trait.
Crates that depend on the collections have been updated and dependencies sorted.
I think I changed all the imports in the tests to make sure it works. I'm not entirely sure, as near the end of the tests there was yet another use that I forgot to change, and when I went to try again, it started rebuilding everything, which I don't currently have time for.

There will probably be incompatibility between this and the other pull requests that are splitting up libextra. I'm happy to rebase once those have been merged.

The tests I didn't get to run should pass. But I can redo them another time if they don't.

alexcrichton · 2014-02-03T08:24:11Z

src/libarena/lib.rs

@@ -23,9 +23,10 @@
 #[feature(managed_boxes)];

 extern mod extra;


I think that this extra dependency could actually be removed now?

huonw · 2014-02-03T08:32:47Z

Maybe the Deque trait can be placed (or reexported) at the top level, since it's a little weird having a single item in a module of the same name.

alexcrichton · 2014-02-04T00:38:38Z

I think that this is a great crate to have. The "collections library" is quite a meaty crate that I imagine will be quite useful to downstream users.

I do have some concerns about this though:

Should this be libcollection or libcollections? It looks like there's java.util.collections and python has a collections library, so perhaps precedent says to go with the pluralization?
I'm a little worried about having some collections in libstd and some in libcollection. I think this is done out of necessity right now, but I would think that it should be a goal to move HashMap to libcollection.
I'm also a little worried about our "stuttering problem". We've solved this in other crates via private modules and reexports at the top level, but that doesn't work quite so well for collections I think. Due to collections frequently having external iterators as their own types, you can't really reexport just TreeMap. That being said, I'd be fine with referring to collections::treemap::Entries, but also referring to collections::TreeMap. I think that the best solution for this for now is to have the reexport, but also have the inner module be public. Our usage and examples of libcollections would have to have use collections::TreeMap instead of the treemap::TreeMap variant.

HeroesGrave · 2014-02-04T05:33:12Z

Man fixing things up is annoying. Just when I think I'm done, I find that now I need to fix up the documentation, which means rebuilding the whole thing over again.

I think I've fixed everything now. I just haven't rebuilt to run the documentation tests.

HeroesGrave · 2014-02-04T05:35:52Z

Anyway, I've fixed up the things you guys mentioned.

Re-exported the main collection types at crate level.
Changed the name to libcollections
Removed the globs from libcollections.
Removed extra dependency from libarena (except for tests)
Ran all the tests except the doc tests which as mentioned in my previous comment, failed and I am currently rebuilding everything again.

Everything should be working now.

huonw · 2014-02-04T05:45:45Z

src/doc/index.md

@@ -38,6 +38,7 @@ li {list-style-type: none; }
 * [The Rust compiler, `librustc`](rustc/index.html)

 * [The `arena` allocation library](arena/index.html)
+* [The 'collections' collection library](collections/index.html)


Needs backticks (), and maybe "The collectionslibrary of generic data-structures" or even just "Thecollections` library"? (Repeating "collection" twice seems a little peculiar.)

Yeah, I was unsure what to put there.

HeroesGrave · 2014-02-04T06:08:09Z

Ran all the tests after a bit of doc editing and everything works fine.

alexcrichton · 2014-02-04T16:52:48Z

src/doc/rust.md


-    use foo::extra::list;  // good: foo is at crate root
+    use foo::collections::list;  // good: foo is at crate root


Let's leave this example as extra for now.

It was an unfortunate collision. I'll change it to extra::time instead.

HeroesGrave · 2014-02-05T04:13:33Z

@alexcrichton: It doesn't seem to automatically inject #[cfg(test)] extern mod extra because when running make check it gives an error that an extern mod extra; is missing.

alexcrichton · 2014-02-05T04:17:15Z

Ah, nevermind then! I guess perhaps there's a test that uses something from extra

HeroesGrave · 2014-02-05T04:17:45Z

The benchmarks need BenchHarness.

HeroesGrave · 2014-02-05T23:17:19Z

@alexcrichton @huonw

I've rebased it against master, and all the tests ran fine.

Now that the other major changes are done (libsync and libserialize), unless there's something else you want, it's ready for merge.

HeroesGrave · 2014-02-06T00:00:20Z

Oops. Forgot to add the fixes for a few tests.

Now it's ready.

HeroesGrave · 2014-02-07T03:34:35Z

@alexcrichton: I've rebased it.

(Was the failure above a fault on my part?)

alexcrichton · 2014-02-07T03:43:57Z

The previous assertion is a bit disturbing, but we'll see what bors has to say this time around.

HeroesGrave · 2014-02-07T05:13:57Z

What the heck is happening?

The log says the test failed because of "unresolved import. maybe a missing extern mod collections", but it does have extern mod collections.

I'm confused.

huonw · 2014-02-07T05:29:49Z

the check-fast driver combines all run-pass tests (that don't have a // xfail-fast directive) into a single crate with each test in a submodule, so use's in them are from the top of the outer module. Two options: use self::collections::...; or just add an // xfail-fast extern mod doesn't work comment after the license.

HeroesGrave · 2014-02-07T06:22:04Z

Good to know it wasn't my fault. :)

I'll get to fixing it now. (and any other tests that will have the same problem)

HeroesGrave · 2014-02-07T06:48:58Z

Fixed. This time I ran make check-fast and at least for me, everything passed.

Part of #8784 Changes: - Everything labeled under collections in libextra has been moved into a new crate 'libcollection'. - Renamed container.rs to deque.rs, since it was no longer 'container traits for extra', just a deque trait. - Crates that depend on the collections have been updated and dependencies sorted. - I think I changed all the imports in the tests to make sure it works. I'm not entirely sure, as near the end of the tests there was yet another `use` that I forgot to change, and when I went to try again, it started rebuilding everything, which I don't currently have time for. There will probably be incompatibility between this and the other pull requests that are splitting up libextra. I'm happy to rebase once those have been merged. The tests I didn't get to run should pass. But I can redo them another time if they don't.

Config revamp Fixes rust-lang/rust-analyzer#11790 Fixes rust-lang/rust-analyzer#12115 This PR changes a lot of config names, and a few ones are being merged or split apart. The reason for this is that our configuration names currently are rather inconsistent and some where poorly chosen in regards to extensability. This PR plans to fix that. We still allow the old config names by patching them to the new ones before deserializing to keep backwards compatability with other clients (the VSCode client will auto update the config) but ideally we will get rid of that layer in the future. Here is a list of the changes: These are simple renames `old_name | alias1 | alias2 ... -> new_name` (the vscode client will fix these up automagically): ``` assist_allowMergingIntoGlobImports -> imports_merge_glob assist_exprFillDefault -> assist_expressionFillDefault assist_importEnforceGranularity -> imports_granularity_enforce assist_importGranularity | assist_importMergeBehavior | assist_importMergeBehaviour -> imports_granularity_group assist_importGroup -> imports_group_enable assist_importPrefix -> imports_prefix cache_warmup -> primeCaches_enable cargo_loadOutDirsFromCheck -> cargo_buildScripts_enable cargo_runBuildScripts | cargo_runBuildScriptsCommand -> cargo_runBuildScripts_overrideCommand cargo_useRustcWrapperForBuildScripts -> cargo_runBuildScripts_useRustcWrapper completion_snippets -> completion_snippets_custom diagnostics_enableExperimental -> diagnostics_experimental_enable experimental_procAttrMacros -> procMacro_attributes_enable highlighting_strings -> semanticHighlighting_strings_enable highlightRelated_breakPoints -> semanticHighlighting_breakPoints_enable highlightRelated_exitPoints -> semanticHighlighting_exitPoints_enable highlightRelated_yieldPoints -> semanticHighlighting_yieldPoints_enable highlightRelated_references -> semanticHighlighting_references_enable hover_documentation -> hover_documentation_enable hover_linksInHover | hoverActions_linksInHover -> hover_links_enable hoverActions_debug -> hoverActions_debug_enable hoverActions_enable -> hoverActions_enable_enable hoverActions_gotoTypeDef -> hoverActions_gotoTypeDef_enable hoverActions_implementations -> hoverActions_implementations_enable hoverActions_references -> hoverActions_references_enable hoverActions_run -> hoverActions_run_enable inlayHints_chainingHints -> inlayHints_chainingHints_enable inlayHints_closureReturnTypeHints -> inlayHints_closureReturnTypeHints_enable inlayHints_hideNamedConstructorHints -> inlayHints_typeHints_hideNamedConstructorHints inlayHints_parameterHints -> inlayHints_parameterHints_enable inlayHints_reborrowHints -> inlayHints_reborrowHints_enable inlayHints_typeHints -> inlayHints_typeHints_enable lruCapacity -> lru_capacity runnables_cargoExtraArgs -> runnables_extraArgs runnables_overrideCargo -> runnables_command rustcSource -> rustc_source rustfmt_enableRangeFormatting -> rustfmt_rangeFormatting_enable ``` These are configs that have been merged or split apart, which have to be manually updated by the user: ``` callInfo_full -> signatureInfo_detail, signatureInfo_documentation_enable cargo_allFeatures, cargo_features -> cargo_features checkOnSave_allFeatures, checkOnSave_features -> checkOnSave_features completion_addCallArgumentSnippets completion_addCallParenthesis -> completion_callable_snippets ```

…-keys, r=Veykril Fix obsolete config keys The config keys were drastically reorganized by rust-lang#12010, but the docs don't reflect the updates, causing inconsistency and confusion. I checked for such obsolete configuration keys and updated to the new one. For reproducibility, I attach a small shell script that I used to examine the old keys. Now the script only detects `cargoExtraArgs` and `overrideCargo`, which originates from other type definition in the code but not from the configuration. <details><summary>script</summary> ```bash echo "allowMergingIntoGlobImports exprFillDefault importEnforceGranularity importGranularity importMergeBehavior importMergeBehaviour importGroup importPrefix warmup loadOutDirsFromCheck runBuildScripts runBuildScriptsCommand useRustcWrapperForBuildScripts enableExperimental procAttrMacros breakPoints exitPoints yieldPoints linksInHover linksInHover gotoTypeDef chainingHints closureReturnTypeHints hideNamedConstructorHints parameterHints reborrowHints typeHints lruCapacity cargoExtraArgs overrideCargo rustcSource enableRangeFormatting assist\.allowMergingIntoGlobImports assist\.exprFillDefault assist\.importEnforceGranularity assist\.importGranularity assist\.importMergeBehavior assist\.importMergeBehaviour assist\.importGroup assist\.importPrefix primeCaches\.enable cache\.warmup cargo\.loadOutDirsFromCheck cargo\.runBuildScripts cargo\.runBuildScriptsCommand cargo\.useRustcWrapperForBuildScripts completion\.snippets diagnostics\.enableExperimental experimental\.procAttrMacros highlighting\.strings highlightRelated\.breakPoints highlightRelated\.exitPoints highlightRelated\.yieldPoints highlightRelated\.references hover\.documentation hover\.linksInHover hoverActions\.linksInHover hoverActions\.debug hoverActions\.enable hoverActions\.gotoTypeDef hoverActions\.implementations hoverActions\.references hoverActions\.run inlayHints\.chainingHints inlayHints\.closureReturnTypeHints inlayHints\.hideNamedConstructorHints inlayHints\.parameterHints inlayHints\.reborrowHints inlayHints\.typeHints lruCapacity runnables\.cargoExtraArgs runnables\.overrideCargo rustcSource rustfmt\.enableRangeFormatting allFeatures addCallArgumentSnippets addCallParenthesis callInfo\.full cargo\.allFeatures checkOnSave\.allFeatures completion\.addCallArgumentSnippets completion\.addCallParenthesis" | while read -r pattern do rg '\b'$pattern'\b([^.]|$)' . -g "!crates/rust-analyzer/src/config/patch_old_style.rs" -g "!editors/code/src/config.ts" -g "!a.sh" --no-heading --color=always --line-number done exit excluded # debug # enable # run # implementations # references # documentation # references # snippets # strings # full ``` </details>

…ng-for-multi-dimension-arrays, r=Alexendoo fix: `manual_memcpy` wrong indexing for multi dimensional arrays fixes: rust-lang#9334 This PR fixes an invalid suggestion for multi-dimensional arrays. For example, ```rust let src = vec![vec![0; 5]; 5]; let mut dst = vec![0; 5]; for i in 0..5 { dst[i] = src[i][i]; } ``` For the above code, Clippy suggests `dst.copy_from_slice(&src[i]);`, but it is not compilable because `i` is only used to loop the array. I adjusted it so that Clippy `manual_memcpy` works properly for multi-dimensional arrays. changelog: [`manual_memcpy`]: Fixes invalid indexing suggestions for multi-dimensional arrays

alexcrichton reviewed Feb 3, 2014
View reviewed changes

huonw reviewed Feb 4, 2014
View reviewed changes

alexcrichton reviewed Feb 4, 2014
View reviewed changes

Arcterus mentioned this pull request Feb 6, 2014

Move getopts out of extra #12007

Merged

moved collections from libextra into libcollections

d81bb44

bors closed this Feb 7, 2014

HeroesGrave deleted the libcollection branch February 7, 2014 21:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move collections from libextra into libcollections #12010

Move collections from libextra into libcollections #12010

HeroesGrave commented Feb 3, 2014

alexcrichton Feb 3, 2014

huonw commented Feb 3, 2014

alexcrichton commented Feb 4, 2014

HeroesGrave commented Feb 4, 2014

HeroesGrave commented Feb 4, 2014

huonw Feb 4, 2014

HeroesGrave Feb 4, 2014

HeroesGrave Feb 4, 2014

HeroesGrave commented Feb 4, 2014

alexcrichton Feb 4, 2014

HeroesGrave Feb 5, 2014

HeroesGrave commented Feb 5, 2014

alexcrichton commented Feb 5, 2014

HeroesGrave commented Feb 5, 2014

HeroesGrave commented Feb 5, 2014

HeroesGrave commented Feb 6, 2014

HeroesGrave commented Feb 7, 2014

alexcrichton commented Feb 7, 2014

HeroesGrave commented Feb 7, 2014

huonw commented Feb 7, 2014

HeroesGrave commented Feb 7, 2014

HeroesGrave commented Feb 7, 2014

		@@ -23,9 +23,10 @@
		#[feature(managed_boxes)];

		extern mod extra;


		use foo::extra::list; // good: foo is at crate root
		use foo::collections::list; // good: foo is at crate root

Move collections from libextra into libcollections #12010

Move collections from libextra into libcollections #12010

Conversation

HeroesGrave commented Feb 3, 2014

alexcrichton Feb 3, 2014

Choose a reason for hiding this comment

huonw commented Feb 3, 2014

alexcrichton commented Feb 4, 2014

HeroesGrave commented Feb 4, 2014

HeroesGrave commented Feb 4, 2014

huonw Feb 4, 2014

Choose a reason for hiding this comment

HeroesGrave Feb 4, 2014

Choose a reason for hiding this comment

HeroesGrave Feb 4, 2014

Choose a reason for hiding this comment

HeroesGrave commented Feb 4, 2014

alexcrichton Feb 4, 2014

Choose a reason for hiding this comment

HeroesGrave Feb 5, 2014

Choose a reason for hiding this comment

HeroesGrave commented Feb 5, 2014

alexcrichton commented Feb 5, 2014

HeroesGrave commented Feb 5, 2014

HeroesGrave commented Feb 5, 2014

HeroesGrave commented Feb 6, 2014

HeroesGrave commented Feb 7, 2014

alexcrichton commented Feb 7, 2014

HeroesGrave commented Feb 7, 2014

huonw commented Feb 7, 2014

HeroesGrave commented Feb 7, 2014

HeroesGrave commented Feb 7, 2014