Generate expressive diagnostic messages with the offending line

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,32 @@ 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-") &&
+ !global.params.mixinOut)
+ {
+ 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,7 +319,7 @@ 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 ))
 
 LEXER_ROOT=$(addsuffix .d, $(addprefix $(ROOT)/, array ctfloat file filename outbuffer port rmem \
  rootobject stringtable hash))

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;
+}