GeezerVison

What's always true?

isinstance(x, StackEffect). It's gone in the next refactor.

Not that it matters yet, but these are almost always fixed-width integer types, not objects (though we'll eventually want handling for objects too):

Just my preference, but I sort of prefer a name like _ to a name like unused for our syntax here. I guess it just feels more "special", and doesn't distract:

Honestly, I don't think we really need the INLINE_CACHE_ENTRIES_WHATEVER stuff (or the asserts it produces) in this file anymore (they were originally added to simplify the JUMPBY(...) moves, but those are going to be generated now).

I feel like it sort of just complicates parsing and code generation for no real benefit... plus, we actually already have asserts to this effect in specialize.c where we ended up re-using these constants in some places.

:(

Hm. This is a bit weird because it treats caches and stack items the same, which doesn't make much sense. It seems to me we'd want something that captures just stack effect and cache size:

(This will probably get more complicated as stack effects start getting more complicated...)

Why is this bad? Seems useful for things like copies and swaps (unless the intention is to have the author assign the output to a new name anyways)?

Maybe it complicates refcounting somehow? Either way, might be worth a comment.

Good question. I thought this was in Mark's DSL spec but I can't find it; I probably just misread something. Intuitively, this rules out cases like

inst(FOO, (left, right -- right)) {
    DECREF(left);
}

which requires shifting right down by one unit, and that seems a bit unexpected (could be caused by a typo?). But you're right, it doesn't cause any complications in the code generator, we'll just generate code like

{
    PyObject *left = PEEK(2), *right = PEEK(1);
    DECREF(left);
    STACK_SHRINK(1);
    POKE(1, right);
}

I'll get rid of this check in the next refactor (it's in the wrong place anyway).

I didn't look too closely at anything below this line (not a parsing expert). I'm sure it works fine, though. :)

I find this control flow easier to follow:

I'm not sure I agree. Your rewrite is more compact but makes it easy to overlook that this code accepts a trailing comma. The longer form makes you pause and notice that.

I noticed that this file has pretty aggressive if nesting. Out of curiousity, any reason why you prefer not to combine many ifs into one test? Maybe it fits your mental model of the parser better?

The latter, mostly. It makes it easier to add an else clause later. Also, I like to test only one condition per line. The parser does need a bit of cleanup, but it's clean enough for now.