Minimal implementation of implicit deref patterns for Strings

[fee1-dead]

cc @compiler-errors @BoxyUwU rust-lang/lang-team#88 #87121

I forgot to add a feature gate, will do so in a minute Done

[rustbot]

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

• Stabilizing library features
• Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
• Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
• Changing public documentation in ways that create new stability guarantees
• Changing observable runtime behavior of library APIs

[rust-highfive]

r? @michaelwoerister

(rust-highfive has picked a reviewer for you, use r? to override)

[BoxyUwU]

lol. lmao. nice

[michaelwoerister]

@fee1-dead, do you know if there's a compiler-team sponsor for this feature?

[compiler-errors]

i think @nikomatsakis originally was sponsoring the attached issue, at least from the lang perspective, though possibly from the compiler side too

[compiler-errors]

ah, nvm, it was @cramertj from the lang side according to the issue

[BoxyUwU]

whats the exchange rate on "compiler-contributor member" to "compiler-team member". does 3 contributors interested count as a compiler team member sponsor? ;-)

[michaelwoerister]

A compiler-contributor member can certainly take this too as long as they feel competent in the areas being modified. I'm mostly looking for someone who has the necessary context.

[rustbot]

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

[flip1995]

Please don't push formatting changes to Clippy.

I wonder why this is happening recently in every other PR 🤔

[fee1-dead]

I wonder why this is happening recently in every other PR thinking

vscode/rust-analyzer has format on save which might not respect clippy's config.

[flip1995]

There are still many Clippy formatting changes in this PR:

- src/tools/clippy/clippy_lints/src/strings.rs
- src/tools/clippy/clippy_lints/src/redundant_clone.rs
- src/tools/clippy/clippy_lints/src/needless_pass_by_value.rs
- src/tools/clippy/clippy_lints/src/methods/string_extend_chars.rs
- src/tools/clippy/clippy_lints/src/methods/search_is_some.rs
- src/tools/clippy/clippy_lints/src/methods/manual_str_repeat.rs
- src/tools/clippy/clippy_lints/src/methods/inefficient_to_string.rs
- src/tools/clippy/clippy_lints/src/matches/match_str_case_mismatch.rs
- src/tools/clippy/clippy_lints/src/manual_retain.rs

[fee1-dead]

yeah I was doing it incrementally

[fee1-dead]

so I reformatted it using clippy's rustfmt config which reverted everything back to normal. The only change it did was to reorder match arms in manual_retain.rs

[fee1-dead]

@rustbot ready

changed to -C panic=abort

[compiler-errors]

@bors r+

[bors]

📌 Commit bc51f87 has been approved by compiler-errors

It is now in the queue for this repository.

[bors]

⌛ Testing commit bc51f87 with merge e07425d...

[bors]

☀️ Test successful - checks-actions
Approved by: compiler-errors
Pushing e07425d to master...

[rust-timer]

Finished benchmarking commit (e07425d): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.7%	[-1.0%, -0.5%]	7
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-0.7%	[-1.0%, -0.5%]	7

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-2.7%	[-2.7%, -2.7%]	1
Improvements ✅ (secondary)	-3.7%	[-3.7%, -3.7%]	1
All ❌✅ (primary)	-2.7%	[-2.7%, -2.7%]	1

Cycles

This benchmark run did not return any relevant results for this metric.

[lqd]

Note for weekly triage: bitmaps is currently noisy

[safinaskar]

@fee1-dead . My expectation was that this PR will convert &T to &T::Target. But it seems that it converts T to &T::Target, this seems to be strange. I. e. with current PR this code works:

  match String::new() {
        "o" => (),
        _ => (),
    }

But this doesn't:

  match &String::new() {
        "o" => (),
        _ => (),
    }

I think that deref patterns should convert &T to &T::Target, because this is exactly thing we need in compilers/interpreters when we expect some deep structure. Say, we have this struct for representing Lisp value:

enum Value {
  Nil,
  Cons(Rc<Value>, Rc<Value>)
}

Then we want this:

let v: Value = ...
match &v {
  Cons(a, Cons(b, Cons(c, d))) => ...
  ...
}

You see? At every step we convert &T to &T::Target. And then again and again

[fee1-dead]

If we have

enum Value {
  Foo, Bar(Option<String>, Vec<i32>),
}

And I want to get the Vec as an owned value when it is Value::Bar(Some("test"), x), then it would only work if we allow deref patterns for owned values.

I get why it might feel confusing, but in reality we are enabling more use cases instead of limiting more. In your case, you can just add & to the "o" pattern to match it.

[estebank]

In your case, you can just add & to the "o" pattern to match it.

It feels like under match ergonomics you shouldn't need to do that, at least once the implementation is more evolved.

[AlbertMarashi]

FYI, this doesn't appear to be working with rust-analyzer. Any workarounds?