Bring more consistency to nmodl::stringutils utility functions about nmodl HOT 6 CLOSED

Even in Option 2 it's not entirely obvious that it's a large performance penalty, due to move semantics. For example:

auto str = trim("   fooo   "); // Create one string, and copy elision.
str = ltrim(rtrim(str)); // 1 copy, 2 moves.
str = ltrim(rtrim(std::move(str))); // 3 moves

Everyone's favourite source suggests that sizeof(std::string) == 3 * sizeof(void*). Hence, a move should copy the name number of bytes for both long and short strings. The source:
https://stackoverflow.com/a/21710033

If you're doing something like stripping whitespace from the front, you end up copying the size of the string (minus the leading whitespace). So you don't have to incur a penalty that larger than the cost of the function.

Finally, there's to_upper which from it's signature is likely used in contexts where one doesn't want to destroy the original. In these cases the copy is unavoidable; and there's no difference in cost between Option 1 and Option 2 in terms of speed.

Which brings us to the final reason why Option 1 is a bit clunky:

trim(str + "foo");  // Option 1 => compilation error
auto str_foo = trim(str + "foo"); // Option 2 => just fine.

from nmodl.

I didn’t check all usages but just want to say that « performance » aspect is not much a concern in the context of nmodl (as its translation program and strings that we manipulate here are hundreds of chars long).

So I don’t have strong: something that make usage simpler, nicer, intuitive is fine.

tagging @1uc as well as he has been looking at the code in recent weeks and would have opinion.

Edit: Copying @1uc's comment from the Slack:

In Option 1 how does one chain operations: ltrim(rtrim(s));

from nmodl.

uff, I'm happy to see that @pramodk had the same feeling as me:

• I don't think either that performance is an issue here. The nmodl codegen is not so performance critical that I would be focusing too much on that
• I much prefer to have an API to use in the codegen code that makes it easy and elegant to write

so my conclusion is that option 1 option 2 is preferable, since functional style allows you to do more and with simpler/cleaner code. Having said this, I absolutely agree with @tristan0x that these functions need to be cleaned up and made consistent.
(edit: I mistakenly wrote option 1, but meant option 2, clearly I should get some more rest before I continue here...)

from nmodl.

One observation, I can't remember seeing any of these functions and I don't immediately see a use for any of them, except tolower / toupper, because they're rather low level. NMODL has stronger abstractions, that I'd expect to deal with padding/alignment issues.

ltrim: 1 use
rtrim: 0 uses + 1 in trim
trim: approx. 16 uses
remove_character: 3 uses
align_text: 3 uses

from nmodl.

ah you're making a good point - but we can clean them up nevertheless.

from nmodl.

It's not an argument against cleaning it up. Rather it's an argument that a super ergonomic API maybe isn't as important as we initially thought. Also the cost of changing this if we notice that it's too slow, isn't prohibitive.

from nmodl.