jmoenig / Snap

a visual programming language inspired by Scratch
http://snap.berkeley.edu
GNU Affero General Public License v3.0
1.51k stars 745 forks source link

Can't report from inside a for loop #481

Closed psafa closed 9 years ago

psafa commented 10 years ago

The following will hang png

psafa commented 10 years ago

This may be related to #250

xtitter commented 10 years ago

"for each" also breaks when containing a reporter, even if it isn't the iterator upvar as below. (Same with 'for' -- any "report" inside blows up.)

image

cycomachead commented 10 years ago

Weird. This very simple case is a regression. I've tested it before and it should work. (It worked at the time of #250.)

cycomachead commented 10 years ago

I noticed this today, and now there's an actual error message attached... screen shot 2014-09-30 at 11 18 36 pm

(Anyone coming across this can start there. :P)

jmoenig commented 10 years ago

I'll be uploading a fix for this (which I've been working on yesterday) shortly. BUT: I'm redesigning REPORT, so this is just going to be workaround for now.

cycomachead commented 10 years ago

:+1:

BUT: I'm redesigning REPORT, so this is just going to be workaround for now.

Oh interesting! In what ways will it work differently?

jmoenig commented 10 years ago

Oh, it's just going to do what the current version should do but doesn't :-) Report out of the nearest enclosing function (reporter, ring or C-slot, whichever is closest), but in the special case of a C-slot reporting out of the enclosing custom block, and - that's the important part - repeating the process recursively, so you can also report out of nested C-shaped custom blocks reliably.

The implementation details are very hairy, especially because of Snap's tail-call-elimination, so I'm playing with various ways to simplify things...

brianharvey commented 10 years ago

When you say "C-slot" I hope you really mean "command type" regardless of C-shaped or inline. I still think (since you're trying to simplify!) that the right thing is simply to report from the closest enclosing procedure that can report. This excludes lambdas in command-type slots, and excludes custom command blocks (e.g., FOR).

The really really right thing is to report from the lexically nearest reportable context, but I remember you telling me that that can't be done, and maybe it isn't necessary. It would be a relevant difference if, say, a C-slot appears in a reporter block (such as the case blocks in Nathan's implementation of switch).

jmoenig commented 10 years ago

No, I'm special casing C-slots. Rings report out of rings, so your MAP implementation works. But maybe I'll come up with something else altogether....

cycomachead commented 10 years ago

Ah ok.

I was playing around with some stuff and noticed this edge case... I'm not exactly sure what's supposed to happen, but I don't think the behavior is currently quite correct. screen shot 2014-10-01 at 1 24 03 am

jmoenig commented 10 years ago

Hmm.... actually this case makes me wonder... it looks nice and feels right...

cycomachead commented 10 years ago

it looks nice and feels right...

I'm not positive...I feel like it doesn't seem right to me, because if anything is below the run the block will report that (or nothing) if they're all command blocks.

OTOH, there is something very 'simple' about this case that makes it seem OK and nice and clean. That text really should have said "Should this appear....?"

brianharvey commented 10 years ago

At first I thought this was an error, but I was wrong. The behavior shown is exactly what should happen. The RUN command is running the REPORT command.

And this clarifies (thank you, @cycomachead) what makes a REPORT in a ringed script so hard to think about: Its interpretation must depend on the context in which it is used. If the TEST procedure had done SAY (CALL {REPORT [some text]}) where braces are rings, then the REPORT should report only from its immediately surrounding ring, because the context expects a reporter ring. But in the original example, the context is RUN, which expects a command ring, and so REPORT has to report from TEST rather than from the ring.

And I believe this corroborates what I said in my previous comment. You report from the nearest enclosing procedure that is allowed to report.

cycomachead commented 10 years ago

The RUN command is running the REPORT command.

But what feels weird to me is that run isn't supposed to ever report anything.

unknown

This also makes sense, I think, considering the way a function should work. It's just that the run block itself causing the reporting of a function doesn't look correct...however, I can possibly see a case where this would be useful.

brianharvey commented 10 years ago

That example should report too.

The right way to think about this, imho, is that runany should always behave as if the command block itself were in the script instead of the RUN block. (Therefore it's not useful to put a fixed block in a RUN unless the RUN is providing inputs to it.)

None of this changes, imho, if the block is REPORT.

cycomachead commented 10 years ago

That example should report too.

It should? It behaves as if there is no explicit report for the block, and reports ''. (Which makes sense, mostly).

should always behave as if the command block itself were in the script instead of the RUN block.

Yeah, that's actually what's happening... Look at this case. I wasn't even sure this was possible: screen shot 2014-10-01 at 2 01 39 pm

In that sense, I guess the current behavior is at least, consistent, so that it good. However, it weirds me out that this kind of works. I would expect the test block to report nothing.

brianharvey commented 10 years ago

That should be an error. I'm sure the behavior you're seeing is an artifact of tail call elimination; the evaluator thinks the REPORT is actually inside the TEST block.

cycomachead commented 10 years ago

That should be an error. I'm sure the behavior you're seeing is an artifact of tail call elimination; the evaluator thinks the REPORT is actually inside the TEST block.

But then why is run with report different? Because once you execute a report, shouldn't you automatically go up a level in the call stack? But in that case, the function execution doesn't end.

brianharvey commented 10 years ago

On 10/1/14 2:12 PM, Michael Ball wrote:

But then why is run with report different?

As I said :-), that's a bug.

cycomachead commented 10 years ago

Ah, ok. :)

cycomachead commented 9 years ago

@jmoenig Seems like these issues have all been solved recently as well! :+1:

jmoenig commented 9 years ago

excellent, thanks for checking @cycomachead