id,summary,reporter,owner,description,type,status,priority,milestone,component,version,severity,resolution,keywords,cc,lang,patch,platform
398,Suspicious code in sub.c - Parrot_capture_lex - Multi,Util,,"In r33193, {{{Parrot_capture_lex}}} was added to src/sub.c, as part of merging the lexicals branch into trunk.
In the section commented as ""MultiSub gets special treatment"", this code originated:
{{{
if (!PMC_IS_NULL(child_sub->outer_sub)) 
    if (0 == string_equal(interp, current_sub->subid, 
                          PMC_sub(child_sub->outer_sub)->subid)) { 
    old = child_sub->outer_ctx; 
    child_sub->outer_ctx = Parrot_context_ref(interp, ctx); 
    if (old) 
        Parrot_free_context(interp, old, 1); 
} 
}}}

The code has three formatting problems. The outer {{{if}}} lacks braces, the closing brace of the middle {{{if}}} is lined up with the outer {{{if}}}, and the code inside the middle {{{if}}} is not indented.
These problems make it too easy to cause inadvertent semantic changes during future modifications, especially by inserting code between the outer and middle {{{if}}}s.

In r37016, I think such a inadvertent semantic change occurred during the change to ATTR accessors.

Either[[BR]]
1. the original code was correct, and now a new codepath is opened when {{{PMC_IS_NULL(child_sub->outer_sub)}}}, or[[BR]]
2. the original code was incorrect in blocking the {{{PMC_IS_NULL(child_sub->outer_sub)}}} codepath, and the problem is now corrected, or[[BR]]
3. my analysis is flawed.

If (2), then the current code needs to change back to the prior codepath, by enclosing the outer {{{if}}} in braces.

If (1) or (2), then {{{Parrot_capture_lex}}} is under-exercised by the test suite. No test detected the change in behavior.

In any case, the code needs to be re-indented.

Other eyes than mine are needed to address this issue; I do not grasp the meaning of the code.
",bug,closed,normal,,core,trunk,medium,fixed,,pmichaud cotto Util,,,