Param scope and body scope should not be merged on this case.

[akroshg]

When the param is used in the function, defined at the param scope, we should be have different param and body scope for the the enclosing function.
However we are using the function id from the top ref of the pidRefStack. This is not correct as we could have pushed a ref on the stack with different block id
but for the same function - so this logic could break.
For fixing that we should walk that pidRefStack to see if there is access to the inner function.

[pleath]

So why isn’t the capture found when we do binding, which pops the stack? It shouldn’t be necessary to walk it like this.

[akroshg]

@pleath Because of deferparsing. As we already merged the param scope and body scope - but while restoring from the scopeinfo we don't have that detail anymore thus the param scope does not have anything (as they were merged already).

[pleath]

I had a feeling. So this is really about the way the stack is reconstructed.

[akroshg]

the pidrefstack is ordered for block id. So we could not find out if the current symbol is used in the nested function.

[pleath]

Binding does find all the non-local references without walking the stack. If there’s a problem that only occurs with deferred parsing, it’s almost certainly because things are not set up correctly when we read the scope info. That could cause other problems besides this one.

[akroshg]

Since we did not correctly find out that param scope and body scope need not merge - that put wrong information while saving the scope info. Later when restore when we did not restore the param scope.
Binding which finds out the non-local references happens later after FinishFncDecl. But the ResetBodyAndParamScopeMerged happens in the ParseFncDeclHelper and before FinishFncDecl.
The top ref in pidRefStack is not ordered based on the functionId so the only way I can find that out is to walk the stack. I thought of reconstructing the stack in the functionId order - but that will slow down the pushing part even in in the case where we don't have default arg.

[pleath]

Okay. That makes sense. So the problem is not with binding or discovery of non-local references, which we do eventually find. It’s having incomplete information when we save scope info. Is that right?

[akroshg]

Yes. Moreover the ResetBodyandParamScopeMerge is using the incomplete information. Which eventually get stored in the scope info.

[pleath]

Is there a reason we can’t wait until the information is complete before saving scope info?

[akroshg]

Looking at the code at ParseFncDeclHelper, it seems we make use of that information up-front to at the body scope creation and push those symbols to the body scope based on if they are needed to merge or not.
@aneeshdk if he can recollect why it was done before binding.