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

Restrict From<S> for {D,Subd}iagnosticMessage. #110579

Merged
merged 1 commit into from
May 3, 2023

Commits on May 2, 2023

  1. Restrict From<S> for {D,Subd}iagnosticMessage.

    Currently a `{D,Subd}iagnosticMessage` can be created from any type that
    impls `Into<String>`. That includes `&str`, `String`, and `Cow<'static,
    str>`, which are reasonable. It also includes `&String`, which is pretty
    weird, and results in many places making unnecessary allocations for
    patterns like this:
    ```
    self.fatal(&format!(...))
    ```
    This creates a string with `format!`, takes a reference, passes the
    reference to `fatal`, which does an `into()`, which clones the
    reference, doing a second allocation. Two allocations for a single
    string, bleh.
    
    This commit changes the `From` impls so that you can only create a
    `{D,Subd}iagnosticMessage` from `&str`, `String`, or `Cow<'static,
    str>`. This requires changing all the places that currently create one
    from a `&String`. Most of these are of the `&format!(...)` form
    described above; each one removes an unnecessary static `&`, plus an
    allocation when executed. There are also a few places where the existing
    use of `&String` was more reasonable; these now just use `clone()` at
    the call site.
    
    As well as making the code nicer and more efficient, this is a step
    towards possibly using `Cow<'static, str>` in
    `{D,Subd}iagnosticMessage::{Str,Eager}`. That would require changing
    the `From<&'a str>` impls to `From<&'static str>`, which is doable, but
    I'm not yet sure if it's worthwhile.
    nnethercote committed May 2, 2023
    Configuration menu
    Copy the full SHA
    6b62f37 View commit details
    Browse the repository at this point in the history