6 ms·

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 us

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?

sanxiyn 6y ago
Clone more.
- ahelwer 6y ago
  See that's seemingly good (and popular) advice, which I didn't find anywhere in the book.
  - steveklabnik 6y ago
    It's in chapter 12 https://doc.rust-lang.org/stable/book/ch12-03-improving-error-handling-and-modularity.html#the-trade-offs-of-using-clone https://doc.rust-lang.org/stable/book/ch12-03-improving-erro...
estebank 6y ago
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 https://doc.rust-lang.org/std/borrow/enum.Cow.html and https://deterministic.space/secret-life-of-cows.html 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 6y ago
  Sure! So I'm trying to implement this feature: https://github.com/tree-sitter/tree-sitter/issues/982#issuecomment-797812264 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-separator 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 6y ago
    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 6y ago
      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.
skohan 6y ago
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
devit 6y ago
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 6y ago

Clone more.

ahelwer 6y ago
See that's seemingly good (and popular) advice, which I didn't find anywhere in the book.
- steveklabnik 6y ago
  It's in chapter 12 https://doc.rust-lang.org/stable/book/ch12-03-improving-error-handling-and-modularity.html#the-trade-offs-of-using-clone https://doc.rust-lang.org/stable/book/ch12-03-improving-erro...

estebank 6y ago

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 https://doc.rust-lang.org/std/borrow/enum.Cow.html and https://deterministic.space/secret-life-of-cows.html 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 6y ago
Sure! So I'm trying to implement this feature: https://github.com/tree-sitter/tree-sitter/issues/982#issuecomment-797812264 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-separator 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 6y ago
  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 6y ago
    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.

skohan 6y ago

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

devit 6y ago

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).