parser: fix exceptions being ignored

[belochub]

• Avoid continuing execution after exception is thrown
  inside a parsing method call.
• Return undefined when possible in cases of exceptions
  raising in all of the methods (to reduce memory usage).

Fixes: #142

[lundibundi]

LGTM, but I'd like if function calls in 'try/catch' used same style - all args in one line with or without newline after equals or each arg on separate line, to make code more consistent and easy to read.

[lundibundi]

Just curious, is this space necessary, as you don't have it in the other places?

[aqrln]

The thing I don't like in this approach is that we basically use JavaScript exceptions instead of C++ ones to handle errors inside the C++ code. How much is the performance overhead, btw? I'd prefer to handle errors in C++ code properly using any pattern you will find appropriate and only throwing JavaScript exceptions from the highest level functions.

[belochub]

@aqrln, well, I haven't checked the performance overhead, but maybe you can do it if you want.
There aren't that many possible solutions to handle this differently, one of which is to just add one or two more arguments to each of the parser functions: the first one indicating whether error has occurred and possibly the second one, used to return the error message if any, but that looks like strangely replicating exceptions functionality without actually using them.
Do you maybe have any other ideas on how to solve this problem in a more elegant way?

[aqrln]

@belochub

but that looks like strangely replicating exceptions functionality without actually using them

Well, that's what Google C++ Style Guide suggests to do, isn't that?

[belochub]

@aqrln, I, probably, should have clarified that I was talking about JS exceptions in the part you quoted, and Google C++ Style Guide gives no opinion on them, because it is out of its scope, obviously.

[aqrln]

@belochub yeah, and my point is that using JavaScript exceptions instead of C++ ones isn't the best way to work around the limitations induced by the styleguide ;)

That said, I do have to admit that we currently have a bug in the parser and this PR resolves the issue in an acceptable (though not very perfect to me) way, so I'm not gonna stand against it. Let's just fix it. I'm okay with discussing the preferred way of handling errors inside the parser internals and refactoring the code later, if there'll be a need to do so.

[belochub]

@aqrln, right, you are saying that it is not a perfect way of fixing it, but do you have a better solution in mind right now?
The way I proposed in previous comments is worse in my opinion, but I also had an idea of somewhat mixed usage of exceptions and returning boolean value indicating that exception has been raised. But it is just another way of avoiding v8::TryCatch and it still uses JavaScript exceptions, thus I don't see much difference with the implementation in this PR.

[aqrln]

@belochub

do you have a better solution in mind right now?

I don't. I would propose it immediately if I did. The point that I'm trying to communicate is just this doesn't feel like quite the best approach to me, but I'm fine with it as long as there are no better alternatives.

[belochub]

Okay then, let's lend this as it is and whenever we get a better solution we will discuss it and refactor this part.

[aqrln]

Landed in 29b3e19. Thanks!