Skip to content

Leverage static str key when possible#727

Merged
KodrAus merged 3 commits into
rust-lang:masterfrom
tisonkun:leverage-static-str-key-when-possible
Jun 1, 2026
Merged

Leverage static str key when possible#727
KodrAus merged 3 commits into
rust-lang:masterfrom
tisonkun:leverage-static-str-key-when-possible

Conversation

@tisonkun
Copy link
Copy Markdown
Contributor

@tisonkun tisonkun commented Jun 1, 2026

Replace &'k str in Key<'k> with MaybeStaticStr<'k> to leverage &'static str when in most cases it is.

Open for review, especially on:

  1. Whether it is backward compatible.
  2. Whether MaybeStaticStr causes significant indirection.

Otherwise, for downstream append, even if the key is a static str, one may have to key.to_string to resolve some lifetime issue. See:

Signed-off-by: tison <wander4096@gmail.com>
Copy link
Copy Markdown
Contributor

@KodrAus KodrAus left a comment

Choose a reason for hiding this comment

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

This looks like a good change to me, thanks @tisonkun!

Comment thread src/kv/key.rs Outdated
Copy link
Copy Markdown
Contributor

@KodrAus KodrAus left a comment

Choose a reason for hiding this comment

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

As noted, we should call the method from_str_static, to match the convention of other methods in log

Comment thread src/macros.rs Outdated
Signed-off-by: tison <wander4096@gmail.com>
@tisonkun tisonkun requested a review from KodrAus June 1, 2026 17:11
Signed-off-by: tison <wander4096@gmail.com>
@tisonkun tisonkun requested a review from Thomasdezeeuw June 1, 2026 17:16
Copy link
Copy Markdown
Contributor

@KodrAus KodrAus left a comment

Choose a reason for hiding this comment

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

Thanks @tisonkun

@KodrAus KodrAus merged commit c906cfb into rust-lang:master Jun 1, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants