Refactor std string extension #8196
Conversation
ext/standard/string.c
Outdated
| if (found) { | ||
| RETURN_LONG(found - ZSTR_VAL(haystack)); | ||
| } else { | ||
| if (!found) { |
There was a problem hiding this comment.
This seems like personal preference, are there any code guidelines where the preferred style is specified?
This moves the happy path below the more uncommon failure path which in theory would cause a pipeline stall. That's probably a non-issue with today's branch prediction. Maybe someone with more processor knowledge could answer that one.
(You could of course also just add UNEXPECTED())
There was a problem hiding this comment.
It is in part, I was trying to unify the style and I do think really nested if/else blocks get confusing which is not the case here. But I have no knowledge about pipelining so if that's an issue I don't mind inverting the conditions again.
There was a problem hiding this comment.
I think if you add an UNEXPECTED it should essentially be equivalent.
Girgias
force-pushed
the
std-string-refactor
branch
from
e5af94b to
a53ea50
Compare
Mainly using more appropriate types, early returns, and moving the happy path to the primary scope
Girgias
force-pushed
the
std-string-refactor
branch
from
a53ea50 to
1979e2b
Compare
Mainly using more appropriate types, early returns, and moving the happy path to the primary scope