hrp: small improvement in hrp parsing

[tcoratger]

In the HRP parsing function, we first test the following conditions with potential error outputs:

if hrp.is_empty() {
    return Err(Empty);
}
if hrp.len() > MAX_HRP_LEN {
    return Err(TooLong(hrp.len()));
}

So we don't need to directly declare the new variable in case an error is thrown and this declaration becoming useless. We can wait for the checks to be ok before declaring this (very small improvement).

[tcharding]

Thanks for taking the time to patch bech32. I'm not an expert in how Rust compiles to assembler but I thought static variables are put on the stack as a part of the function call, so this change has no basically no effect on the stack usage. What other benefits do you think there are?

[apoelstra]

It's not a static variable, it's a local variable -- so in theory the stack "allocation" (which is just incrementing a pointer) will happen later.

In practice I definitely expect the compiler to do this for us. But it's still good practice to do early checks first before making any "allocations", and this will protect us in case we later refactor the Hrp type somehow to do nontrivial things that are less obviously (to the compiler) reorderable.

[apoelstra]

ACK 3322ca4 successfully ran local tests

[tcharding]

Excluding the obvious face palm static vs local. Turns out the thing I was trying to say was this patch doesn't change the stack frame (https://stackoverflow.com/questions/68023230/whats-the-difference-between-stack-pointer-and-frame-pointer-in-assembly-arm)

Anyhow, my point was this is mostly a stylistic change, I was wondering if there was any other benefit I was missing?

ACK 3322ca4

[apoelstra]

@tcharding in the context of a modern compiler it's purely stylistic.

But as your SE link describes, in a naive compilation of the existing code, the stack pointer is moved before doing these checks (and then if they fail it's immediately moved back). While the changed code avoids moving it. The frame pointer, if it exists at all, behaves the same way in both cases.

[tcharding]

Ah nice, thanks.

[clarkmoody]

ACK 3322ca4