The "missing formal parameter" Question

Hi there, this is probably not an issue but I find it somehow strange.

qjs throws an error SyntaxError: missing formal parameter on the following testcase:

function func() {
  var n = new Proxy(Function(), {
      get() {
          console.log(1)
          func()
      }
  })[1]
}

func()

While v8 and jsc will continue execution until the call stack limit is hit.

However, if I remove console.log(1), the error qjs throws becomes SyntaxError: stack overflow, which then becomes consistent with v8/jsc. I wonder what's happening here :)

Best regards and thanks for the great work.

I found throw error in quickjs.c:

            if (s->token.val == '[' || s->token.val == '{') {
                // ...
            } else if (s->token.val == TOK_IDENT) {
                // ...
            } else {
                js_parse_error(s, "missing formal parameter");
                goto fail;
            }

But I don't quite understand the logic here.

I almost know the reason. Else didn't handle it well, because there is another case that the allocable stack size is not enough, so the token value is not handled correctly, and then it goes into the logic of else. For example,

           // s->token.val may actually be ‘(’ or other, because the stack size is not enough for allocation.
           // Then it goes into the logic of else, but not because it is missing formal parameter 
            if (s->token.val == '[' || s->token.val == '{') {
                // ...
            } else if (s->token.val == TOK_IDENT) {
                // ...
            } else {
                js_parse_error(s, "missing formal parameter");
                goto fail;
            }

Therefore, the simplest solution is to increase the stack size to avoid this situation. The best way is to distinguish this situation and handle it, for example:

            if (s->token.val == '[' || s->token.val == '{') {
                // ...
            } else if (s->token.val == TOK_IDENT) {
                // ...
            } else {
                if(js_check_stack_overflow()) {
                      JS_ThrowStackOverflow(ctx);
                } else {
                      js_parse_error(s, "missing formal parameter");
                      goto fail;
                }
            }

I have found the real reason. The exception is overwritten. At first, the exception goes to the stack overflow, the function parsing was not handled properly, and another exception was triggered.

The code is as follows:

int js_parse_function_decl2 (...){
	// ...

	// an error has occurred here, but keep going.
	js_parse_skip_parens_token();

	// because of the previous error, 
	// the token value here is not correct
	// so it goes to else and overwrites the previous error
	if (s->token.val == '[' || s->token.val == '{') {
		// ...
	} else if (s->token.val == TOK_IDENT) {
		// ...
	} else {
		js_parse_error(s, "missing formal parameter");
		goto fail;
	}
}

int js_parse_skip_parens_token() {
	// ...

	// The stack overflows here, but it just breaks.
	if (next_token(s)) {
		/* XXX: should clear the exception generated by next_token() */
		break;
	}
}

@HarlonWang Your analysis and the suggested fix make sense. Thank you for digging into this! :) P.S. If you have time, I think you can submit the fix as a patch to the mailing list (if @bellard thinks it's something worthwhile to fix).

@yype I fixed it by adding simple judgment, The test passed and was quoted in my own project.

            if (s->token.val == '[' || s->token.val == '{') {
                // ...
            } else if (s->token.val == TOK_IDENT) {
                // ...
            } else {
                // Make a judgment here.
                // There may be parsing errors in js_parse_skip_parens_token,
                // so as to avoid the exceptions here covering the actual errors.
                JSValue error = JS_GetException(ctx);
                if (!JS_IsNull(error) && JS_IsError(ctx, error)) {
                    JS_Throw(ctx, error);
                } else {
                    js_parse_error(s, "missing formal parameter");
                }
                goto fail;
            }

I want to submit this fix, but I don't quite understand how to use as a patch to the mailing list, I subscribed to mailing list, Then I received an email, The email reads: list context changed to'quickjs develop'by following command, but I don't know what to do next.

@yype I fixed it by adding simple judgment, The test passed and was quoted in my own project.

            if (s->token.val == '[' || s->token.val == '{') {
                // ...
            } else if (s->token.val == TOK_IDENT) {
                // ...
            } else {
                // Make a judgment here.
                // There may be parsing errors in js_parse_skip_parens_token,
                // so as to avoid the exceptions here covering the actual errors.
                JSValue error = JS_GetException(ctx);
                if (!JS_IsNull(error) && JS_IsError(ctx, error)) {
                    JS_Throw(ctx, error);
                } else {
                    js_parse_error(s, "missing formal parameter");
                }
                goto fail;
            }

I want to submit this fix, but I don't quite understand how to use as a patch to the mailing list, I subscribed to mailing list, Then I received an email, The email reads: list context changed to'quickjs develop'by following command, but I don't know what to do next.

You can just send the git diff to the mailing list. how-to, patch example

I see no problem with your code in the current version.