Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dynamically track the 'static-ness of record strings #344

Merged
merged 1 commit into from
Jul 28, 2019

Conversation

sfackler
Copy link
Member

Logger implementations commonly need to convert a Record into a
non-borrowed form to e.g. log it asynchronously on a separate thread.
This allows them to avoid having to allocate owned Strings for the
module path and file fields, which will in practice almost always be
'static.

cc #206

r? @KodrAus

Logger implementations commonly need to convert a `Record` into a
non-borrowed form to e.g. log it asynchronously on a separate thread.
This allows them to avoid having to allocate owned `String`s for the
module path and file fields, which will in practice almost always be
`'static`.

cc rust-lang#206
@@ -669,6 +669,22 @@ impl LevelFilter {
}
}

#[derive(Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Hash, Debug)]
Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure if we want equality, etc to consider staticness or not. It seems a bit weird for text to not be equal to itself depending on the lifetime, but at the same time it does affect how the Record behaves.

Copy link
Contributor

Choose a reason for hiding this comment

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

Since this is publicly unreachable it seems ok to me. If we ever did want to expose some string-like type we could think about it then?

Copy link
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 good to me!

@@ -669,6 +669,22 @@ impl LevelFilter {
}
}

#[derive(Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Hash, Debug)]
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this is publicly unreachable it seems ok to me. If we ever did want to expose some string-like type we could think about it then?

@KodrAus KodrAus merged commit b0957ea into rust-lang:master Jul 28, 2019
@KodrAus
Copy link
Contributor

KodrAus commented Jul 28, 2019

I'll put together another patch release, since it sounds like this could be blocking folks.

@sfackler sfackler deleted the static-str branch July 28, 2019 03:00
EFanZh pushed a commit to EFanZh/log that referenced this pull request Jul 23, 2023
Co-authored-by: github-actions <github-actions@github.com>
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.

None yet

2 participants