lboasso / oberonc

An Oberon-07 compiler for the JVM
MIT License
147 stars 17 forks source link

How to format errors? #13

Closed johnperry-math closed 5 years ago

johnperry-math commented 5 years ago

I mentioned I would make some suggestions for more helpful error messages. That could be subjective, so I wanted to make sure the following general approach would be OK with you. After all, I don't want to get so far into this that it becomes too hard to undo.

Suppose someone tries to cast two types that don't belong together. Right now the error will be incompatible types or something like that. I thought it might be more helpful to name the types, which leads to the following changes to TypeTest:

  PROCEDURE TypeTest(VAR x: OJG.Item; T: OJB.Type; guard: BOOLEAN);
    VAR xt: OJB.Type; i: INTEGER;
  BEGIN xt := x.type;
    IF (T.form = xt.form) &
       ((T.form = OJB.Pointer) OR
        (T.form = OJB.Record) & (x.mode = OJB.ParStruct)) THEN
      WHILE (xt # T) & (xt # NIL) DO xt := xt.base END ;
      IF xt # T THEN xt := x.type;
        IF xt.form = OJB.Pointer THEN
          IF IsExtension(xt.base, T.base) THEN
            OJG.TypeTest(x, T.base, guard); x.type := T
          ELSE
            i := 0;
            i := Strings.Write(xt.typobj.name, error, i);
            i := Strings.Write(" is not an extension of ", error, i);
            i := Strings.Write(T.typobj.name, error, i);
            OJS.Mark(error)
          END
        ELSIF (xt.form = OJB.Record) & (x.mode = OJB.ParStruct) THEN
          IF IsExtension(xt, T) THEN  OJG.TypeTest(x, T, guard); x.type := T
          ELSE
            i := 0;
            i := Strings.Write(xt.typobj.name, error, i);
            i := Strings.Write(" is not an extension of ", error, i);
            i := Strings.Write(T.typobj.name, error, i);
            OJS.Mark(error)
          END
        ELSE
          i := 0;
          i := Strings.Write(xt.typobj.name, error, i);
          i := Strings.Write(" and ", error, i);
          i := Strings.Write(T.typobj.name, error, i);
          i := Strings.Write(" are incompatible types", error, i);
          OJS.Mark(error)
        END
      ELSIF ~guard THEN OJG.MakeConstItem(x, OJB.boolType, 1)
      END
    ELSE
      i := 0;
      i := Strings.Write("type mismatch for ", error, i);
      i := Strings.Write(xt.typobj.name, error, i);
      i := Strings.Write(" and ", error, i);
      i := Strings.Write(T.typobj.name, error, i);
      OJS.Mark(error)
    END ;
    IF ~guard THEN x.type := OJB.boolType END
  END TypeTest;

Does this sort of thing look OK to you, or would you suggest a different way of going about it?

lboasso commented 5 years ago

Given the goal of this project is to showcase Niklaus Wirth's approach to writing compilers, I'll rather keep the original error message, so that we don't diverge from the Oberon-07 compiler for RISC. I am still OK with small improvements in the error messages that reword obscure errors.

johnperry-math commented 5 years ago

OK, I'm glad I asked. I found these more helpful for me (guessing which variable is causing the problem is sometimes a pain) but I can keep this local to my fork.