Comment author: @gasche

Are you planning to send a github pull request (GPR) to fix this, or should we take care of it? (Is it correct to just use negative numbers in the code produced by this optimization?)

Comment author: vlaviron

I have this fixed as part of my work on exceptions in the backend, but I don't plan to make a specific pull request for this bug.
Using negative numbers in the code fixes the issue, but possibly leads to some regressions in terms of missed optimizations (for example, let (a,b) = (1,2) in a + b would not be simplified into 1 + 2).

Comment author: @alainfrisch

I guess dropping the case for Ltrywith in Matching.map_return would fix the problem.

Is it worth using a more complex logic to maintain the optimization in presence of try-with as in the reported example?

Comment author: vlaviron

I'm not sure it is sound to drop only one case: if an other branch activates the optimization, you would end up with the trywith branch returning a couple and not triggering the handler, which is just plain wrong.
Disabling the optimization by raising an exception in the Ltrywith case would be better, but for (almost) full compatibility you could use two nfail numbers (a positive one and a negative one) and pass two functions f_pos and f_neg to map_return (using the two nfails). You would then call f_pos on the default case, and use ''map_return f_pos f_neg l'' in the recursive calls except the first one in Ltrywith where you would use ''map_return f_neg f_neg l1''.
Nesting the negative catch inside the positive one should allow Simplif.simplify_exits to recover the original code when there are no trywiths.

Or alternatively, you could look at what I'm doing on commit da8a7b4 of #1482: it replaces negative numbers in static exceptions with annotations on Lstaticraise, and as a result the fix gets much cleaner.

Comment author: @alainfrisch

you would end up with the trywith branch returning a couple and not triggering the handler, which is just plain wrong.

I'm not sure to follow. Removing the case for Ltrywith would have the effect of considering the whole try...with sub-expression to be "opaque" w.r.t. the transformation (as e.g. a function call). The sub expression would not be rewritten, and would simply return a tuple or raise an exception (in the same way a function call could raise an exception).

Comment author: vlaviron

Ah, yes, you're right (I don't know why I supposed that it would mean that Ltrywith would get the Lstaticraise treatment).

Comment author: @alainfrisch

FWIW, the test in testsuite/asmcomp/bind_tuples.ml explicitly relies on the optimization going under try...with.

Comment author: @alainfrisch

Working on a fix in simplif.mf, where we keep track of the try..with nesting to avoid inlining a static handler across a try..with.

Comment author: @alainfrisch

#1497