[disinvite]

Part of #509.

Closes #273, #174, #56, #55.

New code parser using the tokenizer components added in #547 and #557.

• This should (hopefully) close all gaps on indenting style.

• The list of tokens is split by each marker sequence: consecutive comment tokens that match the marker pattern. The tokens in between are the "candidates" where we search for the "completion token": the resolution of whatever entity is indicated by the marker.

• The search for a completion token typically ends when we hit a semicolon or curly bracket. However: we do not wait for these "stop tokens" and will use the first code token or comment that qualifies.

• If there is a problem that prevents us from completing a marker sequence, all markers in the sequence get an error assigned to the line of the marker. The goal in our decomps was to have no syntax warning or errors of any kind, so the specifics around error reporting were never really set in stone. Also: the stateful nature of the previous parser made it difficult to "rewind" and flag markers we had already passed.

• Added some minor abstraction around the "extra" options that we can expand in [breaking] refactor marker extras to keys with optional values #550 or its successor.

• Created a lot of new tests scoped to each marker type. We should remove the now-redundant tests in test_parser.py in a follow-up (or now).

• // LINE markers are allowed everywhere, though they are probably only useful inside a function. We could limit these to annotated functions (using our detection of static variables) if needed.

[jonschz]

This looks very promising, but this is also a lot of code. Do you want me to look at anything specific? A full review could take some time.

[disinvite]

This looks very promising, but this is also a lot of code. Do you want me to look at anything specific? A full review could take some time.

Maybe just the new behavior with marker grouping and the mini API on the extra fields to see if they won't create a problem when we return to #550. I'm sure there are variable/function/etc names that could be improved, but I've been looking at this too long so it's hard to judge 😀

I did my usual testing against some of the decomp repos. The only changes are ones we would expect: markers are newly enabled because of improved parsing, or (in only a few cases) disabled because the rules are now more strict. In repos that have parser errors (i.e. the ones using aliases) the line numbers have all changed but the alerts themselves are unchanged, except for cases where we flagged an "unexpected" marker that is now enabled.

[jonschz]

I have looked at the tests in some detail and taken a brief look at the parser code. Nothing major stood out to me, I left a couple of small remarks here and there.

Regarding the MarkerOptions API, I think it's fine for the status quo. I'd rather rebuild it once we have aligned on the goal / vision for #550 instead of investing too much time now.

[jonschz]

I didn't know that we support this, nice! Maybe we should document these annotations in the markdown? Doesn't have to be this PR. I'm also not sure what exactly we need these for, maybe they can be used to e.g. clean up

// STRING: LEGO1 0x100dabb0
static const char* ___crtLCMapStringA_str = "\0";

// STRING: LEGO1 0x100dabb4
static const wchar_t* ___crtLCMapStringA_wstr = L"\0";

in ISLE and RACERS?

[disinvite]

Checking a diff on LEGO1 that did not show up in any of my local tests. (Nice benefit of #587)

Edit: resolved after merging isledecomp/isle#1807.