search

fmidue / logic-tasks

0 stars 1 forks source link

problematische Parser-Rückmeldungen #83

Closed jvoigtlaender closed 5 months ago

jvoigtlaender commented 8 months ago

Zum Beispiel hierzu, bei einer geforderten TreeFormulaAnswer:

C ∧ (¬ D ∧ B) ∧ (¬ A ∨ E)

kommt die Fehlermeldung

C ∧ (¬ D ∧ B) ∧ (¬ A ∨ E)
..............^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

falsches Zeichen: '\8743'
möglich ist hier: space oder end of input
------------------------------------------------------------------------

Mal davon abgesehen, dass es sicher netter wäre, wenn auch in den Fehlermeldungen die Unicode-Symbole verwendet würden, ist das eigentliche Problem hier eins der Klammerung. Denn folgendes würde als korrekt geformte Eingabe akzeptiert:

(C ∧ (¬ D ∧ B)) ∧ (¬ A ∨ E)

Die Fehlermeldung möglich ist hier: space oder end of input hilft diesbezüglich aber überhaupt nicht auf die Sprünge.

Zwei mögliche Lösungen (die zweite davon hat weitere didaktische Aspekte):

owestphal commented 8 months ago

Ich hatte mich vor Weihnachten schon mal daran versucht, das Problem zu lösen. Prinzipiell ist das auch keine große Sache. Der UniversalParser ist nach den Prinzipien in Design Patterns for Parser Combinators aufgebaut. Entsprechend könnte das Problem durch einen zusätzlichen Fehlercontextparser behoben werden (siehe Pattern 4b im Paper):

noFixity :: Parser NoFixity
noFixity = do
  x <- basic
  NoFixity x <$> anyBinaryOp <*> basic <* _noOperator <|> pure (OfBasic x)

_noOperator :: Parser ()
_noOperator =
  notFollowedBy (andParser <|> orParser <|> implicationParser <|> biImplicationParser)
  <|> unexpected (Label $ NonEmpty.fromList "operator (perhaps you are missing some parentheses)")

anstatt https://github.com/fmidue/logic-tasks/blob/57981621e477ee5400bc19821e634149d5547b12/src/UniversalParser.hs#L172-L175

Das Problem ist allerdings, dass dieses Pattern aufgrund von Eigenheiten in der Parsec Implementierung nicht funktioniert. Ich hatte ein Experiment mit Megaparsec gemacht, und da gibt es dann für das Beispiel oben den etwas hiflreicheren Fehler

1:15:
  |
1 | C ∧ (¬ D ∧ B) ∧ (¬ A ∨ E)
  |               ^
unexpected operator (perhaps you are missing some parentheses)
expecting white space

(Das expecting whitespace bekommt man sicher auch noch weg)

Leider ist Parsec aber sehr tief im Autotool verankert, sodass wir für die Aufgabe nicht einfach zu Megaparsec wechseln können. Auch wenn das aus meiner Sicht nach dieser Erfahrung sinvoll wäre.

jvoigtlaender commented 8 months ago

Wie wäre es dann mit folgender Umgehung des Problems?

  1. Der Parser an sich bleibt, wie er ist.
  2. Zusätzlich wird ein Hilfsparser geschrieben, der einfach nur schaut, ob nur legale Token vorkommen. Also dass da ohne Rücksicht auf Klammerung und Reihenfolge/Schachtelung von Symbolen etc. zumindest nur Sachen vorkommen, die prinzipiell in einer Formel erlaubt sind.
  3. Wenn eine Eingabe den eigentlichen Parser besteht, fein.
  4. Wenn sie den eigentlichen Parser nicht besteht, und auch den Hilfsparser nicht, dann passiert nichts weiter über die Fehlermeldung des eigentlichen Parsers hinaus.
  5. Wenn eine Eingabe den eigentlichen Parser nicht besteht, aber den Hilfsparser doch, dann erfolgt zusätzlich zur Fehlermeldung des eigentlichen Parsers noch eine Ausgabe der Art "Bitte vergewissern Sie sich, ob die Anordnung der Symbole den Regeln zur Wohlaufgebautheit von Formeln genügt, und Sie insbesondere genügend Klammern benutzt haben."
owestphal commented 5 months ago

Das Problem mit dem Vorschlag ist, dass wir von Seiten der Aufgaben, den Parser nie aktiv aufrufen. Das macht Autotool für uns. Daher können wir auch nicht einfach zwei Parser hintereindander aufrufen und die Ergebnisse vergleichen. Beider Parser müssten also in einem Parser mittels <|> kombiniert werden. Dann ist allerding das Zurecht-Basteln der Fehlermeldungen nicht so ohne weiteres möglich. Wieder aufgrund von Parsec, das keine konditionale oder generell komplexere über String-Fehler-Messages hinaus gehende Fehlerbehandlung erlaubt. (Laut Megaparsec's Dokumentation bestünde dort die Möglichkeit: "Megaparsec allows us to conditionally process parse errors inside a running parser")

jvoigtlaender commented 5 months ago

Ich verstehe nicht, warum nicht folgendes möglich ist (konzeptionell so ähnlich war es im Grunde ursprünglich in der Bachelorarbeit):

owestphal commented 5 months ago

Ja okay, das geht natürlich. Dann müssen wir aber eigentlich auch nicht diesen Umweg gehen, und können Parsec einfach durch Megaparsec ersetzen.

jvoigtlaender commented 5 months ago

Dann sind wir ein Stück weit wieder zurück (oder auf dem Weg?) zur "zweiten Möglichkeit" aus dem letzten Absatz des Eröffnungskommentars oben, oder?

Zwei mögliche Lösungen (die zweite davon hat weitere didaktische Aspekte):

  • Parser ändern, so dass oben eine andere Fehlermeldung kommt. Und sei es einfach ein allgemeines (bei allen Fehlermeldungen des Parsers) "Vielleicht haben Sie irgendwelche Klammern vergessen?"
  • Beim Einlesen einer TreeFormulaAnswer das Weglassen von Klammern (gemäß Assoziativität) zulassen, also nicht mit Syntaxfehler zurückweisen, aber dann bei der semantischen Bewertung das als Fehler zählen. So war der Aufgabentyp ursprünglich mal, noch im letzten Durchgang, vor Commit bcbacceca1bc7cb2be7d8a62046797ed3c3eddcb (denke ich).
owestphal commented 5 months ago

Wir könnten dann sowohl in partialGrade als auch in completeGrade einen anderen Parser laufen lassen. Damit ließe sich das umsetzen. Der bestehende Parser sollte eigentlich auch beide Varianten unterstützen, sodass das da nur eine andere Struktur Spezifikation angegeben werden müsste.

owestphal commented 5 months ago

Grundsätzliche Architektur umgestetzt durch #125