src: document required else block at src/node_platform.cc

[juanarbol]

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• commit message follows commit guidelines

[addaleax]

This is not redundant because it moves the task out of the InternalCallbackScope in the case in which env is available.

[BridgeAR]

@addaleax since the code is now run after the if statement, does it not behave the same as before?

[addaleax]

@BridgeAR No – this is als the cause of the failures in the WASI tests here, I assume.

More generally, in C++

if (x) {
  some_variable a;
  do_something(); 
} else {
  do_something(); 
}

and

if (x) {
  some_variable a;
}
do_something(); 

are not equivalent if the constructor/destructor of a influence do_something() in some way (which is the case here).

[juanarbol]

Hey @addaleax thank you!! I was asking why it did break WASI tests.

Closing.

[addaleax]

@juanarbol Fwiw, you’re not the first one who ran into this, so it might be worth adding a comment here that this is intentional (especially considering that this PR already had 4 approvals)

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/32913/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/32919/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/33293/

[addaleax]

Landed in 535989e