fix issue_8684 by bailing early in parseArguments

[benjones]

This does make the error messages better for the code from the bug report:

int foo(int n, int m)
{
    int x = foo( 5, m;
    for (int q=0; q<10; ++q){
       ++q;
    }
    return  2;
}

goes from a bunch of lines down to bad.d(3): Error: found ; when expecting , or )

Not sure if this was the best way to approach or implement a fix, so open to feedback.

[dlang-bot]

Thanks for your pull request and interest in making D better, @benjones! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

• My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
• My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
• I have provided a detailed rationale explaining my changes
• New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.

---

If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Auto-close	Bugzilla	Severity	Description
✓	8684	normal	Missing ')' in argument list creates a sea of error messages

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub fetch digger
dub run digger -- build "master + dmd#10529"

[WalterBright]

Seems like most of the generated errors are from check(endtok). This suggests a more general solution of suppressing that if any errors occurred in the lexing process.

[benjones]

The big list of errors came from staying in the while loop and calling check(comma) and trying to parse arguments a bunch of times. The check(endtok) only basically affects whether we correctly parse what comes after a malformed arglist.

I agree there's probably something more general to do with the loop + check(comma) that could be used for for + foreach.

Does the benefit of better diagnostics outweigh the cost of extra checks when parsing?

[rainers]

I'd recommend also adding a more obvious test case.

[benjones]

Good idea @rainers . That changes the behavior, but I think still gives pretty good diagnostics and is simpler code. The test case output that changed previously was unrelated and is improved slightly from before, but is a bit longer than my first attempt which I think is fine.

I added the test case from the bug report.

[rainers]

I think you should not check the error count, as an error from an inner expression can be completely unrelated and might have been recovered from.

[rainers]

As endtok is never a comma, the first part of the condition is unnecessary now. The braces below can also be omitted.
Other than that, LGTM.

[benjones]

is it worth checking for endTok before the loop and switching to while(true)?

[rainers]

A trailing comma is optional, so the check in the while condition should be kept.

[rainers]

Thanks