bellard / quickjs

Public repository of the QuickJS Javascript Engine.
https://bellard.org/quickjs
Other
8.33k stars 867 forks source link

Revert "Add `JS_HasException()` (#265)" #312

Closed bellard closed 2 months ago

bellard commented 4 months ago

This reverts commit db9dbd0a2b6d115c9ef3c0dc37e0c669ef4844e4.

bellard commented 4 months ago

This patch is not correct because "null" is a valid exception. Currently, the only way to test an exception is to test if JS_EXCEPTION is returned. The content of "current_exception" is undefined otherwise.

kasperisager commented 4 months ago

Would it be reasonable to make the contents of current_exception defined at all times so one can actually test for pending exceptions?

bellard commented 4 months ago

nullis a valid exception, so testing for JS_NULL is not correct even if current_exception is defined all the time (try throw null). We may initialize it to JS_UNINITIALIZED instead.

kasperisager commented 4 months ago

That's an awfully good point! Initialising it to JS_UNINITIALIZED instead and letting that mean "no pending exception" sounds like a plan, I can put together a PR.

chqrlie commented 4 months ago

That's an awfully good point! Initialising it to JS_UNINITIALIZED instead and letting that mean "no pending exception" sounds like a plan, I can put together a PR.

Using JS_UNINITIALIZED is indeed a clever way to allow for any valid value to be thrown.

kasperisager commented 4 months ago

@chqrlie Isn't JS_UNDEFINED it's own thing?

chqrlie commented 4 months ago

@chqrlie Isn't JS_UNDEFINED it's own thing?

Yes, I was confused in my earlier comment. JS_UNDEFINED and JS_UNINITIALIZED are different types, JS_UNINITALIZED is used for special let semantics and is a perfect solution for the issue at stake here.