katef / kgt

BNF wrangling and railroad diagrams
BSD 2-Clause "Simplified" License
587 stars 30 forks source link

Incorrect railroad diagram #66

Open aDotInTheVoid opened 11 months ago

aDotInTheVoid commented 11 months ago
$ cat ./eg.ebnf
binding-names = {name, ","}, name, ",", name, [","];
$ ./build/bin/kgt -l iso-ebnf -e rrutf8 < ./eg.ebnf
binding-names:
                      ╭────>────╮
                      │         │
    │├──╭── name ──╮──╯── "," ──╰──┤│
        │          │
        ╰── "," ───╯

However this diagram isn't accurate, as it permits just "name", which isn't allowed by the grammar. The correct output would look like

binding-names:
                                     ╭────>────╮
                                     │         │
    │├──╭── name ──╮── "," ── name ──╯── "," ──╰──┤│
        │          │
        ╰── "," ───╯

This may be related to #61 as they both have very similar inputs.

aDotInTheVoid commented 11 months ago

Looking at this a bit more:

./build/bin/kgt -l iso-ebnf -e dot < ./eg.ebnf | dot -Tpng > dot.png

image

Which looks about right to my untrained eye, although the bottom left "name" not being grey and square may be something. Not sure what this means.

./build/bin/kgt -l iso-ebnf -e rrdot  < ./eg.ebnf | dot -Tpng > rr.png

image

This is defiantly wrong, as it allows doesn't require 2 "name"s

When running with rrd_pretty disabled, it generate's

./build/bin/kgt -l iso-ebnf -e rrdot -n < ./eg.ebnf | dot -Tpng > dot.png

image

which seems right.

This is further confirmed by producing a diagram without rrd_pretty:

$ ./build/bin/kgt -l iso-ebnf -e rrutf8 -n  < ./eg.ebnf
binding-names:
                                                             ╭───────>───────╮
                                                             │               │
    │├─────╭───────────────────────╮── name ── "," ── name ──╯───── "," ─────╰─────┤│
           │                       │
           ╰───── "," ── name ─────╯

which is deffinatly right.

I think the next step is for me to to modify rrd_pretty so I can dump a diagram after each pass, so see which one in the culperate.

aDotInTheVoid commented 11 months ago

I've modified rrd_pretty to dump the output between each pass, to try to find the cuplerit. Annoyingly I couldn't use rrdot for this, because rrdot_output intersperces ast_to_rrd rrd_pretty and actually doing the IO. Instead I've used rrutf8, which while not showing the full AST, does help here.

All output is from 0e405d394a28c1ebb4b6db3d122adc20c9ebad29

$ ./build/bin/kgt -l iso-ebnf -e rrutf8 < ./bad1.ebnf 
Just ran collapse
                                                    ╭────>────╮
                                                    │         │
    │├──╭─────────────────╮── name ── "," ── name ──╯── "," ──╰──┤│
        │                 │
        ╰── "," ── name ──╯

Just ran skippable
                                                    ╭────>────╮
                                                    │         │
    │├──╭─────────────────╮── name ── "," ── name ──╯── "," ──╰──┤│
        │                 │
        ╰── "," ── name ──╯

Just ran redundant
                                                    ╭────>────╮
                                                    │         │
    │├──╭─────────────────╮── name ── "," ── name ──╯── "," ──╰──┤│
        │                 │
        ╰── "," ── name ──╯

Just ran collapse
                                                    ╭────>────╮
                                                    │         │
    │├──╭─────────────────╮── name ── "," ── name ──╯── "," ──╰──┤│
        │                 │
        ╰── "," ── name ──╯

Just ran roll
                                     ╭────>────╮
                                     │         │
    │├──╭── name ──╮── "," ── name ──╯── "," ──╰──┤│
        │          │
        ╰── "," ───╯

Just ran collapse
                                     ╭────>────╮
                                     │         │
    │├──╭── name ──╮── "," ── name ──╯── "," ──╰──┤│
        │          │
        ╰── "," ───╯

Just ran nested
                                     ╭────>────╮
                                     │         │
    │├──╭── name ──╮── "," ── name ──╯── "," ──╰──┤│
        │          │
        ╰── "," ───╯

Just ran ci
                                     ╭────>────╮
                                     │         │
    │├──╭── name ──╮── "," ── name ──╯── "," ──╰──┤│
        │          │
        ╰── "," ───╯

Just ran collapse
                                     ╭────>────╮
                                     │         │
    │├──╭── name ──╮── "," ── name ──╯── "," ──╰──┤│
        │          │
        ╰── "," ───╯

Just ran affixes
                      ╭────>────╮
                      │         │
    │├──╭── name ──╮──╯── "," ──╰──┤│
        │          │
        ╰── "," ───╯

Just ran collapse
                      ╭────>────╮
                      │         │
    │├──╭── name ──╮──╯── "," ──╰──┤│
        │          │
        ╰── "," ───╯

Just ran bottom
                      ╭────>────╮
                      │         │
    │├──╭── name ──╮──╯── "," ──╰──┤│
        │          │
        ╰── "," ───╯

Just ran collapse
                      ╭────>────╮
                      │         │
    │├──╭── name ──╮──╯── "," ──╰──┤│
        │          │
        ╰── "," ───╯

binding-names2:
                      ╭────>────╮
                      │         │
    │├──╭── name ──╮──╯── "," ──╰──┤│
        │          │
        ╰── "," ───╯

This suggests that the problem is in rrd_pretty_affixes

edk0 commented 11 months ago

Yeah, it seems designed to work on a completely rrd_pretty_roll'd tree.

It seems like it would translate

||-->-- A -- B --v-- C -- D -- A -- B --||
    |            |
    ^-- D -- C --<

to

||-->-- A -- B --v--||
    |            |
    ^-- D -- C --<

which would be correct if it incremented the loop counter. Except... that can't ever happen, because rrd_pretty_roll+ would have converted the first diagram to

||-->-- A -- B -- C -- D --v-- A -- B --||
    |                      |
    ^----------------------<

so I'm a bit confused about what it's trying to do. Can you think of an example where it changes anything? (Assuming #67, of course).