Support for Reporting Multiple Compiler Errors

[FrobtheBuilder]

Make the Bebop compiler report multiple errors.

These include validation errors and parsing errors. I had to rework the parser significantly to give it the ability to recover upon invalid input. It's not perfect, as it doesn't retain context well while it is while recovering, causing occasional spurious "not a top level definition!" errors following a syntax error, especially in unions. But it works really well other than that.
When the parser encounters an unexpected token, it attempts to skip forward to the next token that could possibly be valid within the closest context possible. So say, within a definition it will attempt to skip to a token that should start a new field, or if it can't find one, it will skip to the next top level definition. In the worst case, if an exception is thrown all the way to the top level Parse method, it will skip to the end of the current file. I also moved the test schemas around since I wanted to add bebop.json project files for both the valid and invalid schemas, and the compiler always searches recursively so they needed to be separated into different directories.

[andrewmd5]

These should be relative.

[FrobtheBuilder]

Yeah I would have done that, but I'm not sure exactly what it's relative TO and I couldn't find any info on it. Can you enlighten me?

[FrobtheBuilder]

What is the purpose of "DummyDefinition?"

Mainly a contrivance I created so I wouldn't have to make definitions nullable in a bunch of places, but I can do that instead if you'd prefer.

[lynn]

Looks great! I only have some small comments.

My one meta-comment is that there is a bit of this:

bool errored = false;
while (!Eat(TokenKind.CloseBrace)) {
  if (errored && unrecoverable) {
    CancelScope();
    return null;
  }
  errored = false;
  try {
    parse_some_stuff;
    if (wrong) throw new SomeError();
  } catch (SpanException e) {
    _errors.Add(e);
    errored = true;
    SkipUntil(whatever);
    continue;
  }
}

It looks like errored can be eliminated to simplify the control flow a little:

while (!Eat(TokenKind.CloseBrace)) {
  try {
    parse_some_stuff;
    if (wrong) throw new SomeError();
  } catch (SpanException e) {
    _errors.Add(e);
    SkipUntil(whatever);
    if (unrecoverable) {
      CancelScope();
      return null;
    }
    continue;
  }
}

(I would actually be happy if we could eliminate try-catch from the parsing logic entirely, because it feels weird how SpanExceptions are both "things we throw" and "things we make a careful list of" — but then when we do throw one, we're actually immediately catching and putting it in the list… But, I see how it's useful if you have a bunch of parsing steps that could go wrong and you want to recover from them all in the same way. If only C# had monads, or something)

[lynn]

I don't see a good reason for this parameter to become nullable: an error message like The definition 'null' cannot be used as a union branch. with an empty span is not useful. And at the call site, it doesn't look like definition can be null.

[FrobtheBuilder]

You're right. There was a point where I had a call to that without an argument available but it's gone now.

[lynn]

What does "universal follow kinds" mean? Does this mean: if we get confused, we can resume parsing from any token-kind in this set? Maybe it is worthy of a small doc comment.

[FrobtheBuilder]

Yeah, it's the bottom level follow set for pretty much everything. I could add a comment.

[lynn]

I think this could be DRY-er if we just call ExpectAndSkip(new HashSet<TokenKind>{ kind }, additionalTokens, hint), performance be damned (it only affects the very first if, anyway).

[FrobtheBuilder]

Nah the problem there is that will add another error to the log. I could probably just use that SkipUntil method I added though.

[FrobtheBuilder]

Wait nevermind I thought you were looking at a different part.

[lynn]

Maybe just if (kinds.Contains(CurrentToken.Kind)) return;, then un-indent the lines below.

[lynn]

Then IMO this should be called SkipCurrentAndThenSkipUntil.

[FrobtheBuilder]

Maybe it should. It works like that to prevent infinite loops, since it's possible that the current token isn't even potentially valid.