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

@@ -586,6 +586,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

@@ -233,6 +233,31 @@ private void verrorPrint(const ref Loc loc, Color headerColor, const(char)* head
     else
         fputs(tmp.peekString(), stderr);
     fputc('\n', stderr);
+
+    if (global.params.printErrorContext &&
+        // ignore invalid files
+        loc != Loc.initial &&
+        // ignore mixins for now
+        !loc.filename.strstr(".d-mixin-"))
+    {
+        import dmd.filecache : FileCache;
+        auto fllines = FileCache.fileCache.addOrGetFile(loc.filename[0 .. strlen(loc.filename)]);
+
+        if (loc.linnum - 1 < fllines.lines.length)
+        {
+            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,127 @@
+/**
+ * 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, filecache.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 line-by-line representation of a $(REF File, dmd,root,file).
+*/
+class FileAndLines
+{
+    File* file;
+    const(char[])[] lines;
+
+    /**
+    File to read and split into its lines.
+    */
+    this(const(char)[] filename)
+    {
+        file = new File(filename);
+        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 != '\r'; buf++)
+            {
+                if (!*buf)
+                    break;
+            }
+            // handle Windows line endings
+            if (*buf == '\r' && *(buf + 1) == '\n')
+                buf++;
+            lines ~= cast(const(char)[]) prevBuf[0 .. buf - prevBuf];
+            buf++;
+        }
+    }
+
+    void destroy()
+    {
+        if (file)
+        {
+            file.destroy();
+            file = null;
+            lines.destroy();
+            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))
+        {
+            if (payload !is null)
+                return cast(typeof(return)) payload.ptrvalue;
+        }
+
+        auto lines = new FileAndLines(file);
+        files.insert(file, cast(void*) lines);
+        return lines;
+    }
+
+    __gshared fileCache = FileCache();
+
+    // Initializes the global FileCache singleton
+    static __gshared void _init()
+    {
+        fileCache.initialize();
+    }
+
+    void initialize()
+    {
+        files._init();
+    }
+
+    void deinitialize()
+    {
+        foreach (sv; files)
+            sv.destroy();
+        files.reset();
+    }
+}

src/dmd/frontend.d

@@ -53,6 +53,7 @@ void initDMD()
     import dmd.builtin : builtin_init;
     import dmd.dmodule : Module;
     import dmd.expression : Expression;
+    import dmd.filecache : FileCache;
     import dmd.globals : global;
     import dmd.id : Id;
     import dmd.mars : setTarget, addDefaultVersionIdentifiers;
@@ -71,6 +72,7 @@ void initDMD()
     Expression._init();
     Objc._init();
     builtin_init();
+    FileCache._init();
 }
 
 /**

src/dmd/globals.d

@@ -161,6 +161,7 @@ struct Param
     bool vmarkdown;         // list instances of Markdown replacements in Ddoc
 
     bool showGaggedErrors;  // print gagged errors anyway
+    bool printErrorContext;  // 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

@@ -466,6 +466,8 @@ private int tryMain(size_t argc, const(char)** argv)
     Expression._init();
     Objc._init();
     builtin_init();
+    import dmd.filecache : FileCache;
+    FileCache._init();
 
     version(CRuntime_Microsoft)
     {
@@ -1723,6 +1725,10 @@ bool parseCommandLine(const ref Strings arguments, const size_t argc, ref Param
             {
                 params.showGaggedErrors = true;
             }
+            else if (startsWith(p + 9, "context"))
+            {
+                params.printErrorContext = true;
+            }
             else
                 goto Lerror;
         }

src/dmd/root/stringtable.d

@@ -217,6 +217,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;
+    }
+
 private:
 nothrow:
     uint allocValue(const(char)[] str, void* ptrvalue)

src/posix.mak

@@ -319,10 +319,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=context
+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;
+}