[ahelwer]

I mean yeah, that's why I went and read a bunch of the book. But I still run into lots of issues. Like what the hell is a borrow cow and when would I want to use it? And how do I fix the "temporary value dropped when borrowed" error in a series of maps on options?

[skohan]

I would recommend asking questions on the subreddit. There's a standing thread for asking questions, and people usually give high-quality help and respond quickly

[estebank]

If you don't mind sharing snippets for the diagnostics you couldn't figure out, I would love to see them in case we can mechanically suggest the appropriate code, but in general a well placed .clone(), .cloned() or .collect() will be what you want. Likely what's happening is you're iterating over a sequence (you can think of options as a sequence of only one iteration as well), but that iterator is over borrows of the sequence's data, instead of owned values. If you try to return that outside of its enclosing function, you will see that error.

As for a "borrow cow", you probably want to read https://doc.rust-lang.org/std/borrow/enum.Cow.html and https://deterministic.space/secret-life-of-cows.html. For what you are doing, it is an optimization to avoid unnecessary clones that is absolutely not required: make your code work with String and worry about Cow<'_, str> later.

[ahelwer]

Sure! So I'm trying to implement this feature: https://github.com/tree-sitter/tree-sitter/issues/982#issuec...

And here's my branch (you can see the latest commits to see the file I'm modifying): https://github.com/ahelwer/tree-sitter/tree/testfile-separat...

I haven't had a chance to go through and add clones everywhere, and will be away at a PT appointment for the next hour or so, but would appreciate any pointers you can give.

[devit]

It passes cargo check with this change.

  diff --git a/cli/src/test.rs b/cli/src/test.rs
  index ac4807bf..8b9882ab 100644
  --- a/cli/src/test.rs
  +++ b/cli/src/test.rs
  @@ -410,16 +410,17 @@ fn parse_test_content(name: String, content: String, file_path: Option<PathBuf>)
           .map(|b| String::from_utf8_lossy(b).to_string())
           .map(|s| escape_reserved_regex_chars(&s));
       
  -    let suffixHeaderPattern : Option<String> = suffix
  +    let suffixHeaderPattern : Option<String> = suffix.as_ref()
           .map(|s| String::from(r"^===+") + &s + r"\r?\n([^=]*)\r?\n===+" + &s + r"\r?\n");
       
  -    let suffixDividerPattern: Option<String> = suffix
  +    let suffixDividerPattern: Option<String> = suffix.as_ref()
           .map(|s| String::from(r"^---+") + &s + r"\r?\n");
   
  -    let headerRegex = suffixHeaderPattern
  -        .and_then(|s| ByteRegexBuilder::new(&s[..]).multi_line(true).build().ok())
  -        .as_ref()
  -        .unwrap_or(&HEADER_REGEX);
  +    let headerRegexFromSuffixHeaderPattern = suffixHeaderPattern.as_ref()
  +        .and_then(|s| ByteRegexBuilder::new(&s[..]).multi_line(true).build().ok());
  +
  +    let headerRegex = headerRegexFromSuffixHeaderPattern
  +        .as_ref().unwrap_or(&HEADER_REGEX);
   
       // Identify all of the test descriptions using the `======` headers.
       for (header_start, header_end) in headerRegex

[ahelwer]

Thank you for this! the .as_ref() seems to solve a lot of the problems, since I guess just doing a straight .map() takes ownership of the contained string.

[estebank]

The error output for your code is

    error[E0382]: use of moved value: `suffix`
       --> cli/src/test.rs:416:48
        |
    406 |     let suffix = FIRST_HEADER_REGEX
        |         ------ move occurs because `suffix` has type `std::option::Option<std::string::String>`, which does not implement the `Copy` trait
    ...
    414 |         .map(|s| String::from(r"^===+") + &s + r"\r?\n([^=]*)\r?\n===+" + &s + r"\r?\n");
        |          ------------------------------------------------------------------------------- `suffix` moved due to this method call
    415 |
    416 |     let suffixDividerPattern: Option<String> = suffix
        |                                                ^^^^^^ value used here after move
        |
    note: this function consumes the receiver `self` by taking ownership of it, which moves `suffix`
       --> /Users/ekuber/.rustup/toolchains/stable-x86_64-apple-darwin/lib/rustlib/src/rust/library/core/src/option.rs:451:38
        |
    451 |     pub fn map<U, F: FnOnce(T) -> U>(self, f: F) -> Option<U> {
        |                                      ^^^^
    
    error[E0716]: temporary value dropped while borrowed
       --> cli/src/test.rs:419:23
        |
    419 |       let headerRegex = suffixHeaderPattern
        |  _______________________^
    420 | |         .and_then(|s| ByteRegexBuilder::new(&s[..]).multi_line(true).build().ok())
        | |__________________________________________________________________________________^ creates a temporary which is freed while still in use
    421 |           .as_ref()
    422 |           .unwrap_or(&HEADER_REGEX);
        |                                    - temporary value is freed at the end of this statement
    ...
    425 |       for (header_start, header_end) in headerRegex
        |                                         ----------- borrow later used here
        |
        = note: consider using a `let` binding to create a longer lived value

You need to change line 413 to turn `suffix` into an `Option<&str>` to avoid taking ownership of it:

    let suffixHeaderPattern: Option<String> = suffix
        .as_ref()
        .map(...);

and you need to change the `headerRegex` extraction to turn it also into an `Option<&str>` from an `Option<String>` by using `.as_ref()` before the `.and_then` call, which lets you avoid the `&s[..]` reborrow, that only lives until the end of that closure:

    let headerRegex = suffixHeaderPattern
        .as_ref()
        .and_then(|s| ByteRegexBuilder::new(s).multi_line(true).build().ok())
        .unwrap_or(HEADER_REGEX.clone());

Edit: I would also consider this to be a diagnostics bug, for at least the first case rustc should have suggested .as_ref(). For the second part, the compiler would ideally have pointed at the `&s[..]` as part of the problem, and the sibling comment has the change you likely want.

Edit 2: to further drive the point home that this should be a bug, this is the current output for a similar case while I was trying to minimize this:

    error[E0308]: mismatched types
      --> src/main.rs:10:22
       |
    10 |         .map(|s| bar(s));
       |          ---         ^ expected `&Struct`, found struct `Struct`
       |          |
       |          help: consider using `as_ref` instead: `as_ref().map`

https://play.rust-lang.org/?version=stable&mode=debug&editio...

[devit]

The latter change introduces an unnecessary clone(). The correct approach is to instead introduce a variable so that headerRegex can keep being a reference to either that variable or to HEADER_REGEX.

[ahelwer]

Can you expand on how this stops a clone from happening?

[devit]

If you don't call clone() then a clone doesn't happen...

[shepmaster]

> into an `Option<&str>` from an `Option<String>`

`Option::deref` is a good choice here.

[loa_in_]

I have to agree that at the beginning those unfortunate names (Arc, Cow) confused me more than they should, given that I know perfectly well what Copy on Write and reference count means.

If only it was spelled CoW and ARC I would very quickly grasp it (probably).

But it's an extremely minor inconvenience and arguably makes the whole thing very memorable

[devit]

1. You mean Cow<'a, str>? It's an enum of either String or &'a str, typically used with 'static so you can either store an heap-allocated string or a static string without copying it to the heap

2. Given you mention "maps of option", probably you are doing something like opt.map(|x| &x.field) or equivalent which is obviously invalid (since you are returning a reference to a subobject of a function argument). Instead, you need to do opt.as_ref().map(|x| &x.field) (or as_mut() if it's mutable).

[sanxiyn]

Clone more.

[ahelwer]

See that's seemingly good (and popular) advice, which I didn't find anywhere in the book.

[steveklabnik]

It's in chapter 12 https://doc.rust-lang.org/stable/book/ch12-03-improving-erro...