Simplify #3798
Simplify #3798
Conversation
|
|
||
| pub fn is_member(&self) -> bool { | ||
| self.is_member | ||
| pub fn new_non_member(path: PathBuf) -> PackageRoot { |
There was a problem hiding this comment.
Maybe better name is new_foreign?
crates/ra_project_model/src/lib.rs
Outdated
| /// Is a member of the current workspace | ||
| is_member: bool, | ||
| pub is_member: bool, |
There was a problem hiding this comment.
Why pub if we don't mutate it after constructor ?
There was a problem hiding this comment.
Right, unfortunately, I cannot put readonly in front of a property. This is pub, so that we don't clone the .path property here and there but just move it out of the temporary PackageRoot. Though, the special deconstructor method for this purpose would seem out-of-place...
There was a problem hiding this comment.
Thats why we use accessor method in the first place.
There was a problem hiding this comment.
And there is no significant different between a.foo and a.foo() if foo() return a ref.
There was a problem hiding this comment.
The difference is that you cannot move out values from the ref. E.g.:
fn consume(p: PathBuf) {};
consume(a.foo);
consume(a.foo()); // cannot move out of shared referenceThere was a problem hiding this comment.
Yes, but we only move out once, and I don't think it is so significant.
| @@ -87,7 +91,7 @@ fn test_globs() { | |||
|
|
|||
| let filter = RustPackageFilterBuilder::default() | |||
| .set_member(true) | |||
| .exclude(Glob::new("src/llvm-project/**").unwrap()) | |||
| .exclude(std::iter::once(Glob::new("src/llvm-project/**").unwrap())) | |||
There was a problem hiding this comment.
By the way, this also works since impl IntoIter for Option holds:
.exclude(Some(Glob::new("src/llvm-project/**").unwrap()))But it looks very hacky :D
There was a problem hiding this comment.
.exclude([Glob::new("src/llvm-project/**")])?
There was a problem hiding this comment.
Nope, there is not into_iter for [T; N]
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
&[T] yields only refs &T, but here we need T
|
nice cleanup, but needs a rebase bors d+ |
|
✌️ Veetaha can now approve this pull request. To approve and merge a pull request, simply reply with |
|
bors r+ |
Build succeeded |
|
@matklad I think the issue with |
…, r=saethlin miri-script: use --remap-path-prefix to print errors relative to the right root Inspired by rust-lang/rust-clippy#13232, this makes it so that when cargo-miri fails to build, `./miri check` will print errors with paths like `cargo-miri/src/setup.rs`. That means we can get rid of the miri-symlink-hacks and instead tell RA to just always invoke the `./miri clippy` script just once, in the root. This means that we can no longer share a target dir between cargo-miri and miri as the RUSTFLAGS are different to crates that are shared in the dependency tree need to be built twice with two different flags. `miri-script` hence now has to set the MIRI environment variable to tell the `cargo miri setup` invocation where to find Miri. I also made it so that errors in miri-script itself are properly shown in RA, for which the `./miri` shell wrapper needs to set the right flags.
bear with me