Generate expressive diagnostic messages

Author: wilzbach

changelog/error-context.dd

@@ -0,0 +1,20 @@
+dmd now supports expressive diagnostic error messages with `-verrors=context`
+
+With the new CLI option `-verrors=context` dmd will now show the offending line directly in its error messages.
+Consider this faulty program `test.d`:
+
+---
+void foo()
+{
+    a = 1;
+}
+---
+
+Now run it with `-verrors=context`:
+
+$(CONSOLE
+> dmd -verrors=context test.d
+test.d(4): $(RED Error): undefined identifier a
+    a = 1;
+    ^
+)

dub.sdl

@@ -22,6 +22,7 @@ subPackage {
     "src/dmd/console.d" \
     "src/dmd/entity.d" \
     "src/dmd/errors.d" \
+    "src/dmd/filecache.d" \
     "src/dmd/globals.d" \
     "src/dmd/id.d" \
     "src/dmd/identifier.d" \

src/dmd/cli.d

@@ -553,6 +553,9 @@ dmd -cov -unittest myprog.d
         Option("verrors=spec",
             "show errors from speculative compiles such as __traits(compiles,...)"
         ),
+        Option("verrors=context",
+            "show error messages with the context of the erroring source line"
+        ),
         Option("-version",
             "print compiler version and exit"
         ),

src/dmd/errors.d

@@ -236,6 +236,31 @@ private void verrorPrint(const ref Loc loc, Color headerColor, const(char)* head
     else
         fputs(tmp.peekString(), stderr);
     fputc('\n', stderr);
+
+    if (global.params.contextPrintErrors &&
+        // ignore invalid files
+        loc != Loc.initial &&
+        // ignore mixins for now
+        !loc.filename.strstr(".d-mixin-"))
+    {
+        import dmd.filecache : fileCache;
+        auto fllines = fileCache.addOrGetFile(loc.filename[0 .. strlen(loc.filename)]);
+
+        if (loc.linnum - 1 < fllines.lines.dim)
+        {
+            auto line = (*fllines.lines)[loc.linnum - 1];
+            if (loc.charnum < line.length)
+            {
+                fprintf(stderr, "%.*s\n", line.length, line.ptr);
+                foreach (_; 1 .. loc.charnum)
+                    fputc(' ', stderr);
+
+                fputc('^', stderr);
+                fputc('\n', stderr);
+            }
+        }
+    }
+end:
     fflush(stderr);     // ensure it gets written out in case of compiler aborts
 }
 

src/dmd/filecache.d

@@ -0,0 +1,140 @@
+/**
+ * Compiler implementation of the
+ * $(LINK2 http://www.dlang.org, D programming language).
+ *
+ * Copyright:   Copyright (C) 1999-2018 by The D Language Foundation, All Rights Reserved
+ * Authors:     $(LINK2 http://www.digitalmars.com, Walter Bright)
+ * License:     $(LINK2 http://www.boost.org/LICENSE_1_0.txt, Boost License 1.0)
+ * Source:      $(LINK2 https://github.com/dlang/dmd/blob/master/src/dmd/filecache.d, _errors.d)
+ * Documentation:  https://dlang.org/phobos/dmd_filecache.html
+ * Coverage:    https://codecov.io/gh/dlang/dmd/src/master/src/dmd/filecache.d
+ */
+
+module dmd.filecache;
+
+import dmd.root.stringtable;
+import dmd.root.array;
+import dmd.root.file;
+
+import core.stdc.stdio;
+
+// A simple wrapper around a string in D for extern(C++) compatibility
+extern(C++) struct Dstring
+{
+    char* ptr;
+    size_t length;
+    extern(D) string toString()
+    {
+        return cast(string) ptr[0 .. length];
+    }
+}
+
+/**
+A line-by-line representation of a $(REF File, dmd, root, file).
+*/
+class FileAndLines
+{
+    File* file;
+    Array!Dstring* lines;
+
+    /**
+    File to read and split into its lines.
+    */
+    this(const(char)* filename)
+    {
+        file = File.create(filename);
+        lines = new Array!Dstring;
+        readAndSplit();
+    }
+
+    // Read a file and split the file buffer linewise
+    private void readAndSplit()
+    {
+        file.read();
+        auto buf = file.buffer;
+        // slice into lines
+        while (*buf)
+        {
+            auto prevBuf = buf;
+            for (; *buf != '\n'; buf++)
+            {
+                if (!*buf)
+                    break;
+            }
+            if (buf != prevBuf)
+                lines.push(Dstring(cast(char*) prevBuf, buf - prevBuf));
+            buf++;
+        }
+    }
+
+    void destroy()
+    {
+        if (file)
+        {
+            import core.memory : GC;
+            file.__dtor();
+            GC.free(file);
+            file = null;
+            lines.__dtor();
+            GC.free(lines);
+            lines = null;
+        }
+    }
+    ~this()
+    {
+        destroy();
+    }
+}
+
+/**
+A simple file cache that can be used to avoid reading the same file multiple times.
+It stores its cached files as $(LREF FileAndLines)
+*/
+struct FileCache
+{
+    private StringTable files;
+
+    /**
+    Add or get a file from the file cache.
+    If the file isn't part of the cache, it will be read from the filesystem.
+    If the file has been read before, the cached file object will be returned
+
+    Params:
+        file = file to load in (or get from) the cache
+
+    Returns: a $(LREF FileAndLines) object containing a line-by-line representation of the requested file
+    */
+    FileAndLines addOrGetFile(const(char)[] file)
+    {
+        if (auto payload = files.lookup(file.ptr, file.length))
+            if (payload !is null)
+                return cast(typeof(return)) payload.ptrvalue;
+
+        auto lines = new FileAndLines(file.ptr);
+        auto p = files.insert(file.ptr, file.length, cast(void*) lines);
+        return cast(typeof(return)) p.ptrvalue;
+    }
+
+    void initialize()
+    {
+        files._init();
+    }
+
+    void deinitialize()
+    {
+        foreach (sv; files)
+            sv.destroy();
+        files.reset();
+    }
+}
+
+static fileCache = FileCache();
+
+shared static this()
+{
+    fileCache.initialize();
+}
+shared static ~this()
+{
+    fileCache.deinitialize();
+}

src/dmd/globals.d

@@ -158,6 +158,7 @@ struct Param
      */
 
     bool showGaggedErrors;  // print gagged errors anyway
+    bool contextPrintErrors;  // print errors with the error context (the error line in the source file)
     bool manual;            // open browser on compiler manual
     bool usage;             // print usage and exit
     bool mcpuUsage;         // print help on -mcpu switch

src/dmd/mars.d

@@ -1732,6 +1732,10 @@ private bool parseCommandLine(const ref Strings arguments, const size_t argc, re
                 {
                     params.showGaggedErrors = true;
                 }
+                else if (startsWith(p + 9, "context"))
+                {
+                    params.contextPrintErrors = true;
+                }
                 else
                     goto Lerror;
             }

src/dmd/root/stringtable.d

@@ -168,6 +168,20 @@ public:
         return 0;
     }
 
+    extern(D) int opApply(scope int delegate(const(StringValue)*) dg)
+    {
+        foreach (const se; table[0 .. tabledim])
+        {
+            if (!se.vptr)
+                continue;
+            const sv = getValue(se.vptr);
+            int result = dg(sv);
+            if (result)
+                return result;
+        }
+        return 0;
+    }
+
     StringValue* update(const(char)[] name) nothrow
     {
         return update(name.ptr, name.length);

src/posix.mak

@@ -307,10 +307,10 @@ FRONT_SRCS=$(addsuffix .d, $(addprefix $D/,access aggregate aliasthis apply argt
 	typinf utils scanelf scanmach statement_rewrite_walker statementsem staticcond safe blockexit printast \
 	semantic2 semantic3))
 
-LEXER_SRCS=$(addsuffix .d, $(addprefix $D/, console entity errors globals id identifier lexer tokens utf))
+LEXER_SRCS=$(addsuffix .d, $(addprefix $D/, console entity errors filecache globals id identifier lexer tokens utf root/stringtable root/file))
 
-LEXER_ROOT=$(addsuffix .d, $(addprefix $(ROOT)/, array ctfloat file filename outbuffer port rmem \
-	rootobject stringtable hash))
+LEXER_ROOT=$(addsuffix .d, $(addprefix $(ROOT)/, array ctfloat filename outbuffer port rmem \
+	rootobject hash))
 
 ROOT_SRCS = $(addsuffix .d,$(addprefix $(ROOT)/,aav array ctfloat file \
 	filename man outbuffer port response rmem rootobject speller \

src/win32.mak

@@ -165,7 +165,7 @@ FRONT_SRCS=$D/access.d $D/aggregate.d $D/aliasthis.d $D/apply.d $D/argtypes.d $D
 	$D/libmscoff.d $D/scanmscoff.d $D/statement_rewrite_walker.d $D/statementsem.d $D/staticcond.d \
 	$D/semantic2.d $D/semantic3.d
 
-LEXER_SRCS=$D/console.d $D/entity.d $D/errors.d $D/globals.d $D/id.d $D/identifier.d \
+LEXER_SRCS=$D/console.d $D/entity.d $D/errors.d $D/filecache.d $D/globals.d $D/id.d $D/identifier.d \
 	$D/lexer.d $D/tokens.d $D/utf.d
 
 LEXER_ROOT=$(ROOT)/array.d $(ROOT)/ctfloat.d $(ROOT)/file.d $(ROOT)/filename.d \

test/fail_compilation/fail_pretty_errors.d

@@ -0,0 +1,36 @@
+/*
+REQUIRED_ARGS: -verrors=pretty
+TEST_OUTPUT:
+---
+fail_compilation/fail_pretty_errors.d(20): Error: undefined identifier `a`
+    a = 1;
+    ^
+fail_compilation/fail_pretty_errors.d-mixin-25(25): Error: undefined identifier `b`
+fail_compilation/fail_pretty_errors.d(30): Error: cannot implicitly convert expression `5` of type `int` to `string`
+    string x = 5;
+               ^
+fail_compilation/fail_pretty_errors.d(35): Error: mixin `fail_pretty_errors.testMixin2.mixinTemplate!()` error instantiating
+    mixin mixinTemplate;
+    ^
+---
+*/
+
+void foo()
+{
+    a = 1;
+}
+
+void testMixin1()
+{
+    mixin("b = 1;");
+}
+
+mixin template mixinTemplate()
+{
+    string x = 5;
+}
+
+void testMixin2()
+{
+    mixin mixinTemplate;
+}