Skip to content
This repository has been archived by the owner on Mar 23, 2023. It is now read-only.

Implement generator.throw() #369

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

m4ns0ur
Copy link
Contributor

@m4ns0ur m4ns0ur commented Jul 23, 2017

No description provided.

@@ -379,6 +379,10 @@ def visit_Yield(self, node):
self.writer.write('return {}, nil'.format(value.expr))
self.writer.write_label(resume_label)
result = self.block.alloc_temp()
self.writer.write(textwrap.dedent("""\
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@trotterdylan not sure if try/except block is able to catch this

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you want to do:

if πRaised != nil {
	πE = πRaised
	πRaised = nil
	if πE != nil {
		continue
	}
}

You could also merge the write() call below into this:

self.writer.write_tmpl(textwrap.dedent("""\
    if πRaised != nil {
    \tπE = πRaised
    \tπRaised = nil
    \tif πE != nil {
    \t\tcontinue
    \t}
    }
    $result = πSent"""), result=result.name)

raised = f.RaiseType(TypeErrorType, "can't send non-None value to a just-started generator")
} else {
func (g *Generator) resume(f *Frame, sendValue *Object, raisedValue *BaseException) (*Object, *BaseException) {
if raisedValue == nil {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@trotterdylan do we still need to update g.state while having throw?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think you can remove this check and update g.state normally for throw.

@m4ns0ur
Copy link
Contributor Author

m4ns0ur commented Jul 23, 2017

@trotterdylan what is the best approach to test stmt and expr_visitor? could you give me a hint?

Copy link
Contributor

@trotterdylan trotterdylan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

edit: sorry, accidentally submitted before I was finished

Thinking about this some more, maybe it's better to test in testing/generator_test.py instead of trying to do this in a stmt_test.py.

compiler/stmt.py Outdated
@@ -590,9 +590,14 @@ def visit_function_inline(self, node):
self.writer.write('var πE *πg.BaseException; _ = πE')
if func_block.is_generator:
self.writer.write(
'return πg.NewGenerator(πF, func(πSent *πg.Object) '
'return πg.NewGenerator(πF, '
'func(πSent *πg.Object, πRaised *πg.BaseException) '
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe call it πThrown

@@ -379,6 +379,10 @@ def visit_Yield(self, node):
self.writer.write('return {}, nil'.format(value.expr))
self.writer.write_label(resume_label)
result = self.block.alloc_temp()
self.writer.write(textwrap.dedent("""\
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you want to do:

if πRaised != nil {
	πE = πRaised
	πRaised = nil
	if πE != nil {
		continue
	}
}

You could also merge the write() call below into this:

self.writer.write_tmpl(textwrap.dedent("""\
    if πRaised != nil {
    \tπE = πRaised
    \tπRaised = nil
    \tif πE != nil {
    \t\tcontinue
    \t}
    }
    $result = πSent"""), result=result.name)


func generatorThrow(f *Frame, args Args, _ KWArgs) (*Object, *BaseException) {
argc := len(args)
if argc == 1 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can leave out the argc == 1 and argc > 4 checks here. If argc < 2 || argc > 4 then the checkMethodArgs will fail because it won't match expectedTypes. The error message isn't ideal in that case, but I think the simplicity warrants it.

raised = f.RaiseType(TypeErrorType, "can't send non-None value to a just-started generator")
} else {
func (g *Generator) resume(f *Frame, sendValue *Object, raisedValue *BaseException) (*Object, *BaseException) {
if raisedValue == nil {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think you can remove this check and update g.state normally for throw.

@m4ns0ur m4ns0ur changed the title [WIP] Implement generator.throw() Implement generator.throw() Jul 26, 2017
@m4ns0ur
Copy link
Contributor Author

m4ns0ur commented Jul 26, 2017

@trotterdylan generator cannot catch the thrown exception, I should check more.

@m4ns0ur
Copy link
Contributor Author

m4ns0ur commented Aug 24, 2017

@trotterdylan sorry for the long delay.

so the problem is generator cannot catch the thrown exception.
here is the CPython code (in generator test):

def gen8():
  try:
    yield
  except ValueError as e:
    assert "foo" in str(e)
    yield
  else:
    raise AssertionError
g = gen8()
g.next()
g.throw(ValueError, 'foo')

and Grumpy generated code:

.
.
.
for ; πF.State() >= 0; πF.PopCheckpoint() {
	switch πF.State() {
	case 0:
	case 2: goto Label2
	case 3: goto Label3
	case 5: goto Label5
	default: panic("unexpected function state")
	}
	// line 2: try:
	πF.SetLineno(2)
	πF.PushCheckpoint(2)
	// line 3: yield
	πF.SetLineno(3)
	πF.PushCheckpoint(3)
	return πg.None, nil
Label3:
	if πThrown != nil {
		πE = πThrown
		πThrown = nil
		if πE != nil {
			continue
		}
	}
	πTemp001 = πSent
	πF.PopCheckpoint()
	if πTemp001, πE = πg.ResolveGlobal(πF, ßAssertionError); πE != nil {
		continue
	}
	// line 8: raise AssertionError
	πF.SetLineno(8)
	πE = πF.Raise(πTemp001, nil, nil)
	continue
	goto Label1
Label2:
	if πE == nil {
	  continue
	}
	πE = nil
	πTemp002, πTemp003 = πF.ExcInfo()
	if πTemp001, πE = πg.ResolveGlobal(πF, ßValueError); πE != nil {
		continue
	}
	if πTemp004, πE = πg.IsInstance(πF, πTemp002.ToObject(), πTemp001); πE != nil {
		continue
	}
	if πTemp004 {
		goto Label4
	}
	πE = πF.Raise(πTemp002.ToObject(), nil, πTemp003.ToObject())
	continue
	// line 4: except ValueError as e:
	πF.SetLineno(4)
Label4:
	µe = πTemp002.ToObject()
	// line 5: assert "foo" in str(e)
	πF.SetLineno(5)
	πTemp005 = πF.MakeArgs(1)
	if πE = πg.CheckLocal(πF, µe, "e"); πE != nil {
		continue
	}
	πTemp005[0] = µe
	if πTemp006, πE = πg.ResolveGlobal(πF, ßstr); πE != nil {
		continue
	}
	if πTemp007, πE = πTemp006.Call(πF, πTemp005, nil); πE != nil {
		continue
	}
	πF.FreeArgs(πTemp005)
	if πTemp004, πE = πg.Contains(πF, πTemp007, ßfoo.ToObject()); πE != nil {
		continue
	}
	πTemp001 = πg.GetBool(πTemp004).ToObject()
	if πE = πg.Assert(πF, πTemp001, nil); πE != nil {
		continue
	}
	// line 6: yield
	πF.SetLineno(6)
	πF.PushCheckpoint(5)
	return πg.None, nil
Label5:
	if πThrown != nil {
		πE = πThrown
		πThrown = nil
		if πE != nil {
			continue
		}
	}
	πTemp001 = πSent
	πF.RestoreExc(nil, nil)
	goto Label1
Label1:
}
.
.
.

it seems OK to me, do you see anything improper?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants