From fcea25c9bbb316505c99bf793ed97aad0d6084a4 Mon Sep 17 00:00:00 2001
From: Sam Vervaeck <samvv@pm.me>
Date: Wed, 24 Aug 2022 20:57:26 +0200
Subject: [PATCH] Improve diagnostic output

---
 CMakeLists.txt               |  3 ++
 include/bolt/CST.hpp         |  6 +---
 include/bolt/Checker.hpp     |  5 +--
 include/bolt/Diagnostics.hpp | 18 +++++++----
 include/bolt/Scanner.hpp     | 10 +++---
 include/bolt/String.hpp      |  4 +--
 include/bolt/Text.hpp        | 14 ++++++---
 src/CST.cc                   |  8 ++++-
 src/Checker.cc               |  4 +--
 src/Diagnostics.cc           | 59 +++++++++++++++++++++++++++++++-----
 src/Parser.cc                | 22 +++++++-------
 src/Scanner.cc               | 15 ++++-----
 src/Text.cc                  |  4 +--
 src/main.cc                  | 13 +++-----
 14 files changed, 122 insertions(+), 63 deletions(-)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index f3821d1dc..7c40686e8 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -8,8 +8,10 @@ set(CMAKE_CXX_STANDARD 17)
 add_subdirectory(deps/zen EXCLUDE_FROM_ALL)
 
 set(ICU_DIR "${CMAKE_CURRENT_SOURCE_DIR}/build/icu/install")
+set(ICU_CFLAGS "-DUNISTR_FROM_CHAR_EXPLICIT=explicit -DUNISTR_FROM_STRING_EXPLICIT=explicit -DU_NO_DEFAULT_INCLUDE_UTF_HEADERS=1 -DU_HIDE_OBSOLETE_UTF_OLD_H=1")
 set(ICU_INCLUDE_DIRS "${ICU_DIR}/include")
 set(ICU_LIBRARY_DIRS "${ICU_DIR}/lib")
+set(ICU_LIBRARIES icuuc)
 
 add_library(
   BoltCore
@@ -29,6 +31,7 @@ target_compile_options(
   BoltCore
   PUBLIC
   -fstandalone-debug
+  ${ICU_CFLAGS}
 )
 target_include_directories(
   BoltCore
diff --git a/include/bolt/CST.hpp b/include/bolt/CST.hpp
index 5dff45223..f590515b4 100644
--- a/include/bolt/CST.hpp
+++ b/include/bolt/CST.hpp
@@ -122,11 +122,7 @@ namespace bolt {
       return StartLoc;
     }
 
-    inline TextLoc getEndLoc() {
-      TextLoc EndLoc;
-      EndLoc.advance(getText());
-      return EndLoc;
-    }
+    TextLoc getEndLoc();
 
     inline size_t getStartLine() {
       return StartLoc.Line;
diff --git a/include/bolt/Checker.hpp b/include/bolt/Checker.hpp
index 7505566a8..dcced3829 100644
--- a/include/bolt/Checker.hpp
+++ b/include/bolt/Checker.hpp
@@ -228,9 +228,10 @@ namespace bolt {
 
     Type* Left;
     Type* Right;
+    Node* Source;
 
-    inline CEqual(Type* Left, Type* Right):
-      Constraint(ConstraintKind::Equal), Left(Left), Right(Right) {}
+    inline CEqual(Type* Left, Type* Right, Node* Source = nullptr):
+      Constraint(ConstraintKind::Equal), Left(Left), Right(Right), Source(Source) {}
 
   };
 
diff --git a/include/bolt/Diagnostics.hpp b/include/bolt/Diagnostics.hpp
index eac875f05..59d357ced 100644
--- a/include/bolt/Diagnostics.hpp
+++ b/include/bolt/Diagnostics.hpp
@@ -40,22 +40,24 @@ namespace bolt {
   class UnexpectedTokenDiagnostic : public Diagnostic {
   public:
 
+    TextFile& File;
     Token* Actual;
     std::vector<NodeType> Expected;
 
-    inline UnexpectedTokenDiagnostic(Token* Actual, std::vector<NodeType> Expected):
-      Diagnostic(DiagnosticKind::UnexpectedToken), Actual(Actual), Expected(Expected) {}
+    inline UnexpectedTokenDiagnostic(TextFile& File, Token* Actual, std::vector<NodeType> Expected):
+      Diagnostic(DiagnosticKind::UnexpectedToken), File(File), Actual(Actual), Expected(Expected) {}
 
   };
 
   class UnexpectedStringDiagnostic : public Diagnostic {
   public:
 
+    TextFile& File;
     TextLoc Location;
     String Actual;
 
-    inline UnexpectedStringDiagnostic(TextLoc Location, String Actual):
-      Diagnostic(DiagnosticKind::UnexpectedString), Location(Location), Actual(Actual) {}
+    inline UnexpectedStringDiagnostic(TextFile& File, TextLoc Location, String Actual):
+      Diagnostic(DiagnosticKind::UnexpectedString), File(File), Location(Location), Actual(Actual) {}
 
   };
 
@@ -75,9 +77,10 @@ namespace bolt {
     
     Type* Left;
     Type* Right;
+    Node* Source;
 
-    inline UnificationErrorDiagnostic(Type* Left, Type* Right):
-      Diagnostic(DiagnosticKind::UnificationError), Left(Left), Right(Right) {}
+    inline UnificationErrorDiagnostic(Type* Left, Type* Right, Node* Source):
+      Diagnostic(DiagnosticKind::UnificationError), Left(Left), Right(Right), Source(Source) {}
 
   };
 
@@ -116,6 +119,9 @@ namespace bolt {
 
     void setForegroundColor(Color C);
     void setBackgroundColor(Color C);
+    void setBold(bool Enable);
+    void setItalic(bool Enable);
+    void setUnderline(bool Enable);
     void resetStyles();
 
     void writeGutter(
diff --git a/include/bolt/Scanner.hpp b/include/bolt/Scanner.hpp
index e2e6a6d40..435a5c251 100644
--- a/include/bolt/Scanner.hpp
+++ b/include/bolt/Scanner.hpp
@@ -24,11 +24,11 @@ namespace bolt {
 
   };
 
-  template<typename ContainerT>
-  class VectorStream : public Stream<typename ContainerT::value_type> {
+  template<typename ContainerT, typename T = typename ContainerT::value_type>
+  class VectorStream : public Stream<T> {
   public:
 
-    using value_type = typename ContainerT::value_type;
+    using value_type = T;
 
     ContainerT& Data;
     value_type Sentry;
@@ -82,6 +82,8 @@ namespace bolt {
 
   class Scanner : public BufferedStream<Token*> {
 
+    TextFile& File;
+
     Stream<Char>& Chars;
 
     TextLoc CurrLoc;
@@ -111,7 +113,7 @@ namespace bolt {
 
   public:
 
-    Scanner(Stream<Char>& Chars);
+    Scanner(TextFile& File, Stream<Char>& Chars);
 
   };
 
diff --git a/include/bolt/String.hpp b/include/bolt/String.hpp
index 7164d712e..1a0ed767d 100644
--- a/include/bolt/String.hpp
+++ b/include/bolt/String.hpp
@@ -5,9 +5,9 @@
 
 namespace bolt {
 
-  using Char = char32_t;
+  using Char = int;
 
-  using String = std::basic_string<Char>;
+  using String = std::string;
 
 }
 
diff --git a/include/bolt/Text.hpp b/include/bolt/Text.hpp
index a576a618a..a92a438bf 100644
--- a/include/bolt/Text.hpp
+++ b/include/bolt/Text.hpp
@@ -17,7 +17,7 @@ namespace bolt {
     size_t Line = 1;
     size_t Column = 1;
 
-    inline void advance(const std::string& Text) {
+    inline void advance(const String& Text) {
       for (auto Chr: Text) {
         if (Chr == '\n') {
           Line++;
@@ -28,6 +28,12 @@ namespace bolt {
       }
     }
 
+    inline TextLoc operator+(const String& Text) const {
+      TextLoc Out { Line, Column };
+      Out.advance(Text);
+      return Out;
+    }
+
   };
 
   class TextRange {
@@ -39,13 +45,13 @@ namespace bolt {
   class TextFile {
 
     ByteString Path;
-    String Text;
+    ByteString Text;
 
     std::vector<size_t> LineOffsets;
 
   public:
 
-    TextFile(ByteString Path, String Text);
+    TextFile(ByteString Path, ByteString Text);
 
     size_t getLine(size_t Offset);
     size_t getColumn(size_t Offset);
@@ -55,7 +61,7 @@ namespace bolt {
 
     ByteString getPath() const;
 
-    String getText() const;
+    ByteString getText() const;
 
   };
 
diff --git a/src/CST.cc b/src/CST.cc
index 46a5ce149..66d2c76f9 100644
--- a/src/CST.cc
+++ b/src/CST.cc
@@ -9,7 +9,7 @@ namespace bolt {
     auto CurrNode = this;
     for (;;) {
       if (CurrNode->Type == NodeType::SourceFile) {
-        return static_cast<SourceFile*>(this);
+        return static_cast<SourceFile*>(CurrNode);
       }
       CurrNode = CurrNode->Parent;
       ZEN_ASSERT(CurrNode != nullptr);
@@ -23,6 +23,12 @@ namespace bolt {
     };
   }
 
+  TextLoc Token::getEndLoc() {
+    auto EndLoc = StartLoc;
+    EndLoc.advance(getText());
+    return EndLoc;
+  }
+
   void Token::setParents() {
   }
 
diff --git a/src/Checker.cc b/src/Checker.cc
index 1bc0e80e3..850ff637f 100644
--- a/src/Checker.cc
+++ b/src/Checker.cc
@@ -203,7 +203,7 @@ namespace bolt {
         std::vector<Type*> ArgTys;
         ArgTys.push_back(inferExpression(Y->LHS, Ctx));
         ArgTys.push_back(inferExpression(Y->RHS, Ctx));
-        Ctx.addConstraint(new CEqual { new TArrow(ArgTys, RetTy), OpTy });
+        Ctx.addConstraint(new CEqual { new TArrow(ArgTys, RetTy), OpTy, X });
         return RetTy;
       }
 
@@ -257,7 +257,7 @@ namespace bolt {
         {
           auto Y = static_cast<CEqual*>(Constraint);
           if (!unify(Y->Left, Y->Right, Sub)) {
-            DE.add<UnificationErrorDiagnostic>(Y->Left, Y->Right);
+            DE.add<UnificationErrorDiagnostic>(Y->Left, Y->Right, Y->Source);
           }
           break;
         }
diff --git a/src/Diagnostics.cc b/src/Diagnostics.cc
index df1417bc9..f3c29d31f 100644
--- a/src/Diagnostics.cc
+++ b/src/Diagnostics.cc
@@ -2,6 +2,7 @@
 #include <sstream>
 #include <cmath>
 
+#include "bolt/CST.hpp"
 #include "zen/config.hpp"
 
 #include "bolt/Diagnostics.hpp"
@@ -201,6 +202,24 @@ namespace bolt {
     }
   }
 
+  void ConsoleDiagnostics::setBold(bool Enable) {
+    if (Enable) {
+      Out << ANSI_BOLD;
+    }
+  }
+
+  void ConsoleDiagnostics::setItalic(bool Enable) {
+    if (Enable) {
+      // TODO
+    }
+  }
+
+  void ConsoleDiagnostics::setUnderline(bool Enable) {
+    if (Enable) {
+      Out << ANSI_UNDERLINE;
+    }
+  }
+
   void ConsoleDiagnostics::resetStyles() {
     if (EnableColors) {
       Out << ANSI_RESET;
@@ -311,7 +330,14 @@ namespace bolt {
       case DiagnosticKind::UnexpectedToken:
       {
         auto E = static_cast<const UnexpectedTokenDiagnostic&>(D);
-        Out << "<unknown.bolt>:" << E.Actual->getStartLine() << ":" << E.Actual->getStartColumn() << ": expected ";
+        setForegroundColor(Color::Red);
+        setBold(true);
+        Out << "error: ";
+        resetStyles();
+        setForegroundColor(Color::Yellow);
+        Out << E.File.getPath() << ":" << E.Actual->getStartLine() << ":" << E.Actual->getStartColumn() << ":";
+        resetStyles();
+        Out << " expected ";
         switch (E.Expected.size()) {
           case 0:
             Out << "nothing";
@@ -322,7 +348,7 @@ namespace bolt {
           default:
             auto Iter = E.Expected.begin();
             Out << describe(*Iter++);
-            NodeType Prev;
+            NodeType Prev = *Iter++;
             while (Iter != E.Expected.end()) {
               Out << ", " << describe(Prev);
               Prev = *Iter++;
@@ -330,15 +356,20 @@ namespace bolt {
             Out << " or " << describe(Prev);
             break;
         }
-        Out << " but instead got '" << E.Actual->getText() << "'\n";
-        writeExcerpt(E.Actual->getSourceFile()->getTextFile(), E.Actual->getRange(), E.Actual->getRange(), Color::Red);
+        Out << " but instead got '" << E.Actual->getText() << "'\n\n";
+        writeExcerpt(E.File, E.Actual->getRange(), E.Actual->getRange(), Color::Red);
+        Out << "\n";
         break;
       }
 
       case DiagnosticKind::UnexpectedString:
       {
         auto E = static_cast<const UnexpectedStringDiagnostic&>(D);
-        Out << "<unknown.bolt>:" << E.Location.Line << ":" << E.Location.Column << ": unexpected '";
+        setForegroundColor(Color::Red);
+        setBold(true);
+        Out << "error: ";
+        resetStyles();
+        Out << E.File.getPath() << ":" << E.Location.Line << ":" << E.Location.Column << ": unexpected '";
         for (auto Chr: E.Actual) {
           switch (Chr) {
             case '\\':
@@ -352,14 +383,28 @@ namespace bolt {
               break;
           }
         }
+        Out << "'\n\n";
+        TextRange Range { E.Location, E.Location + E.Actual };
+        writeExcerpt(E.File, Range, Range, Color::Red);
+        Out << "\n";
         break;
       }
 
       case DiagnosticKind::UnificationError:
       {
         auto E = static_cast<const UnificationErrorDiagnostic&>(D);
-        Out << ANSI_FG_RED << ANSI_BOLD << "error: " << ANSI_RESET << "the types " << ANSI_FG_GREEN << describe(E.Left) << ANSI_RESET
-            << " and " << ANSI_FG_GREEN << describe(E.Right) << ANSI_RESET << " failed to match\n";
+        setForegroundColor(Color::Red);
+        setBold(true);
+        Out << "error: ";
+        resetStyles();
+        Out << "the types " << ANSI_FG_GREEN << describe(E.Left) << ANSI_RESET
+            << " and " << ANSI_FG_GREEN << describe(E.Right) << ANSI_RESET << " failed to match\n\n";
+        if (E.Source) {
+          auto Range = E.Source->getRange();
+          //std::cerr << Range.Start.Line << ":" << Range.Start.Column << "-" << Range.End.Line << ":" << Range.End.Column << "\n";
+          writeExcerpt(E.Source->getSourceFile()->getTextFile(), Range, Range, Color::Red);
+          Out << "\n";
+        }
         break;
       }
 
diff --git a/src/Parser.cc b/src/Parser.cc
index 08615b7af..debf4218b 100644
--- a/src/Parser.cc
+++ b/src/Parser.cc
@@ -70,7 +70,7 @@ namespace bolt {
   { \
     auto __Token = Tokens.get(); \
     if (__Token->Type != NodeType::name) { \
-      throw UnexpectedTokenDiagnostic(__Token, std::vector<NodeType> { NodeType::name }); \
+      throw UnexpectedTokenDiagnostic(File, __Token, std::vector<NodeType> { NodeType::name }); \
     } \
   }
 
@@ -81,7 +81,7 @@ namespace bolt {
         Tokens.get();
         return new BindPattern(static_cast<Identifier*>(T0));
       default:
-        throw UnexpectedTokenDiagnostic(T0, std::vector { NodeType::Identifier });
+        throw UnexpectedTokenDiagnostic(File, T0, std::vector { NodeType::Identifier });
     }
   }
 
@@ -89,18 +89,18 @@ namespace bolt {
     std::vector<Identifier*> ModulePath;
     auto Name = Tokens.get();
     if (Name->Type != NodeType::Identifier) {
-      throw UnexpectedTokenDiagnostic(Name, std::vector { NodeType::Identifier });
+      throw UnexpectedTokenDiagnostic(File, Name, std::vector { NodeType::Identifier });
     }
     for (;;) {
       auto T1 = Tokens.peek();
-      if (T1->Type == NodeType::Dot) {
+      if (T1->Type != NodeType::Dot) {
         break;
       }
       Tokens.get();
       ModulePath.push_back(static_cast<Identifier*>(Name));
       Name = Tokens.get();
       if (Name->Type != NodeType::Identifier) {
-        throw UnexpectedTokenDiagnostic(Name, std::vector { NodeType::Identifier });
+        throw UnexpectedTokenDiagnostic(File, Name, std::vector { NodeType::Identifier });
       }
     }
     return new QualifiedName(ModulePath, static_cast<Identifier*>(Name));
@@ -112,7 +112,7 @@ namespace bolt {
       case NodeType::Identifier:
         return new ReferenceTypeExpression(parseQualifiedName());
       default:
-        throw UnexpectedTokenDiagnostic(T0, std::vector { NodeType::Identifier });
+        throw UnexpectedTokenDiagnostic(File, T0, std::vector { NodeType::Identifier });
     }
   }
 
@@ -129,7 +129,7 @@ namespace bolt {
         Tokens.get();
         return new ConstantExpression(T0);
       default:
-        throw UnexpectedTokenDiagnostic(T0, std::vector { NodeType::Identifier, NodeType::IntegerLiteral });
+        throw UnexpectedTokenDiagnostic(File, T0, std::vector { NodeType::Identifier, NodeType::IntegerLiteral, NodeType::StringLiteral });
     }
   }
 
@@ -200,16 +200,16 @@ namespace bolt {
 
   LetDeclaration* Parser::parseLetDeclaration() {
 
-    PubKeyword* Pub;
+    PubKeyword* Pub = nullptr;
     LetKeyword* Let;
-    MutKeyword* Mut;
+    MutKeyword* Mut = nullptr;
     auto T0 = Tokens.get();
     if (T0->Type == NodeType::PubKeyword) {
       Pub = static_cast<PubKeyword*>(T0);
       T0 = Tokens.get();
     }
     if (T0->Type != NodeType::LetKeyword) {
-      throw UnexpectedTokenDiagnostic(T0, std::vector { NodeType::LetKeyword });
+      throw UnexpectedTokenDiagnostic(File, T0, std::vector { NodeType::LetKeyword });
     }
     Let = static_cast<LetKeyword*>(T0);
     auto T1 = Tokens.peek();
@@ -277,7 +277,7 @@ after_params:
           // First tokens of Pattern
           Expected.push_back(NodeType::Identifier);
         }
-        throw UnexpectedTokenDiagnostic(T2, Expected);
+        throw UnexpectedTokenDiagnostic(File, T2, Expected);
     }
 
     BOLT_EXPECT_TOKEN(LineFoldEnd);
diff --git a/src/Scanner.cc b/src/Scanner.cc
index 47a570cd4..c17403523 100644
--- a/src/Scanner.cc
+++ b/src/Scanner.cc
@@ -3,6 +3,7 @@
 
 #include "zen/config.hpp"
 
+#include "bolt/Text.hpp"
 #include "bolt/Integer.hpp"
 #include "bolt/CST.hpp"
 #include "bolt/Diagnostics.hpp"
@@ -65,8 +66,8 @@ namespace bolt {
     { "mod", NodeType::ModKeyword },
   };
 
-  Scanner::Scanner(Stream<Char>& Chars):
-    Chars(Chars) {}
+  Scanner::Scanner(TextFile& File, Stream<Char>& Chars):
+    File(File), Chars(Chars) {}
 
   Token* Scanner::read() {
 
@@ -225,7 +226,7 @@ digit_finish:
               case '\'': Text.push_back('\''); break;
               case '"': Text.push_back('"'); break;
               default:
-                throw UnexpectedStringDiagnostic(Loc, String { C1 });
+                throw UnexpectedStringDiagnostic(File, Loc, String { static_cast<char>(C1) });
             }
             Escaping = false;
           } else {
@@ -252,7 +253,7 @@ after_string_contents:
           getChar();
           auto C2 = peekChar();
           if (C2 == '.') {
-            throw UnexpectedStringDiagnostic(getCurrentLoc(), String { C2 });
+            throw UnexpectedStringDiagnostic(File, getCurrentLoc(), String { static_cast<char>(C2) });
           }
           return new DotDot(StartLoc);
         }
@@ -304,11 +305,7 @@ after_string_contents:
     BOLT_SIMPLE_TOKEN('}', RBrace)
 
     default:
-
-      throw UnexpectedStringDiagnostic(StartLoc, String { C0 });
-
-      // TODO Add a diagnostic message indicating that scanning failed.
-      //return new Invalid(StartLoc);
+      throw UnexpectedStringDiagnostic(File, StartLoc, String { static_cast<char>(C0) });
 
     }
 
diff --git a/src/Text.cc b/src/Text.cc
index ce1d0233e..ab2e8f968 100644
--- a/src/Text.cc
+++ b/src/Text.cc
@@ -6,7 +6,7 @@
 
 namespace bolt {
 
-  TextFile::TextFile(ByteString Path, String Text):
+  TextFile::TextFile(ByteString Path, ByteString Text):
     Path(Path), Text(Text) {
       LineOffsets.push_back(0);
       for (size_t I = 0; I < Text.size(); I++) {
@@ -46,7 +46,7 @@ namespace bolt {
     return Path;
   }
 
-  String TextFile::getText() const {
+  ByteString TextFile::getText() const {
     return Text;
   }
 
diff --git a/src/main.cc b/src/main.cc
index 1ba875f52..9523f83c7 100644
--- a/src/main.cc
+++ b/src/main.cc
@@ -14,10 +14,10 @@
 
 using namespace bolt;
 
-String readFile(std::string Path) {
+ByteString readFile(std::string Path) {
 
   std::ifstream File(Path);
-  String Out;
+  ByteString Out;
 
   File.seekg(0, std::ios::end);   
   Out.reserve(File.tellg());
@@ -40,22 +40,19 @@ int main(int argc, const char* argv[]) {
 
   auto Text = readFile(argv[1]);
   TextFile File { argv[1], Text };
-  VectorStream<String> Chars(Text, EOF);
-  Scanner S(Chars);
+  VectorStream<ByteString, Char> Chars(Text, EOF);
+  Scanner S(File, Chars);
   Punctuator PT(S);
   Parser P(File, PT);
 
   SourceFile* SF; 
 
-#ifdef NDEBUG
   try {
     SF = P.parseSourceFile();
   } catch (Diagnostic& D) {
     DE.addDiagnostic(D);
+    return 1;
   }
-#else
-  SF = P.parseSourceFile();
-#endif
 
   SF->setParents();