Allow creating a Proj from an owned String

[TomFryersMidsummer]

CString::new takes any Into<Vec<u8>>, which includes String. This should make it possible to avoid reallocating when passing in an owned String with sufficient capacity.

[TomFryersMidsummer]

The test failures were caused by some doctests calling

Proj::new_known_crs(&from, &to, None)

where from is &str, so &from is &&str. Perhaps there is more code out there that relies on this working.

[TomFryersMidsummer]

I think the TryFrom implementation extension doesn't cause breakage, since that never accepted &&str.

[michaelkirk]

I think the existing type signature is going to be more intuitive for the caller. If I'm understanding your motivation, having to type in a & before your owned string does not seem like a big enough hurdle to justify this change.

Unless I'm misunderstanding, I don't think we should merge this.

[michaelkirk]

Sorry, I missed your initial comment that this is about avoiding re-allocation inside of CString. But my conclusion is the same - it seems unlikely that gains here will be worth having a more opaque/confusing input type.

Are you experiencing this part of the code as a bottleneck?

[TomFryersMidsummer]

I just find it a bit odd for Proj to ask for a reference if in the end it wants to own its input. (See C-CALLER-CONTROL.)

Having had another look, I don't think this is performance-critical for us, so I don't mind if you don't think it's worth it.

The type could also be Into<String>, which is clearer and more type-safe, but less flexible.

[lnicola]

FWIW, in gdal we take &str because:

• if you can measure the performance difference, you're almost certainly doing something wrong
• S: Into<Vec<u8>> is not a type I'd want our users to see, especially given that most of them will be coming from other languages, like Python
• pushing the NUL byte will usually cause a reallocation anyway

[michaelkirk]

I appreciate the attempt, but I think on balance this is not worthwhile, so closing.