Refactor lookupPath to avoid recursion. NFC#23017
Conversation
hoodmane
force-pushed
the
lookup-path-nonrecursive
branch
from
b959b5b to
1611617
Compare
sbc100
left a comment
sbc100
left a comment
There was a problem hiding this comment.
Thanks for working on this!
| // split the absolute path | ||
| var parts = path.split('/').filter((p) => !!p); | ||
| // limit max consecutive symlinks to 40 (SYMLOOP_MAX). | ||
| linkloop: for (var nlinks = 0; nlinks < 40; nlinks ++) { |
There was a problem hiding this comment.
Wow, labeled continue targets!
sbc100
left a comment
sbc100
left a comment
There was a problem hiding this comment.
Can you rebase the codesize tests?
|
Well it's going to conflict with #22998, I suppose you can pick which one to merge first. |
|
Can you confirm those codesize changes are all due to this change? i.e. can you run the rebase on |
|
There are a lot of similar changes when I run it on main too. |
Maybe try with |
|
Even after |
Hmm.. that is strange/annoying. Maybe just revert those files then, and I can do a rebaseline after this lands. |
sbc100
changed the title
|
OK, I just pushed another rebaseline commit, perhaps this one will match what you are generated on main now? |
|
Nope, even if I check out your rebaseline commit and do a clean install of tot from emsdk I get a bunch of changes in code size by one to ten bytes. |
|
I think this PR fixes the case when you symlink a directory to itself |
|
@sbc100 can this be merged? |
|
Thanks! |
This removes the recursion from `lookupPath` and in my opinion makes the control flow much more explicit and comprehensible.
2 participants
This removes the recursion from
lookupPathand in my opinion makes the control flow much more explicit and comprehensible.