From 835a240bcb7dcd01a1201f7ca3fe822cfe4364a9 Mon Sep 17 00:00:00 2001 From: Sebastian Wilzbach Date: Wed, 11 Jul 2018 06:38:05 +0200 Subject: [PATCH] Generate expressive diagnostic messages --- changelog/error-context.dd | 20 +++ dub.sdl | 1 + src/dmd/cli.d | 3 + src/dmd/errors.d | 25 ++++ src/dmd/filecache.d | 137 +++++++++++++++++++++ src/dmd/globals.d | 1 + src/dmd/mars.d | 4 + src/dmd/root/stringtable.d | 14 +++ src/posix.mak | 6 +- src/win32.mak | 2 +- test/fail_compilation/fail_pretty_errors.d | 36 ++++++ 11 files changed, 245 insertions(+), 4 deletions(-) create mode 100644 changelog/error-context.dd create mode 100644 src/dmd/filecache.d create mode 100644 test/fail_compilation/fail_pretty_errors.d diff --git a/changelog/error-context.dd b/changelog/error-context.dd new file mode 100644 index 000000000000..b74a2e254dc3 --- /dev/null +++ b/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; + ^ +) diff --git a/dub.sdl b/dub.sdl index bb6179b79f2d..a6bfe3169860 100644 --- a/dub.sdl +++ b/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" \ diff --git a/src/dmd/cli.d b/src/dmd/cli.d index 12030092a02e..4dd2ea361952 100644 --- a/src/dmd/cli.d +++ b/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" ), diff --git a/src/dmd/errors.d b/src/dmd/errors.d index 0c7c4fe9604d..b56b5d520aa2 100644 --- a/src/dmd/errors.d +++ b/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 } diff --git a/src/dmd/filecache.d b/src/dmd/filecache.d new file mode 100644 index 000000000000..31f0cdbd1d32 --- /dev/null +++ b/src/dmd/filecache.d @@ -0,0 +1,137 @@ +/** + * 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) + { + 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.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(); +} diff --git a/src/dmd/globals.d b/src/dmd/globals.d index 4d4a4481507c..e51982ac7140 100644 --- a/src/dmd/globals.d +++ b/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 diff --git a/src/dmd/mars.d b/src/dmd/mars.d index 0f8e977be63f..e77ff53686c0 100644 --- a/src/dmd/mars.d +++ b/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; } diff --git a/src/dmd/root/stringtable.d b/src/dmd/root/stringtable.d index 0452e791f3e1..a1f9889e4e44 100644 --- a/src/dmd/root/stringtable.d +++ b/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); diff --git a/src/posix.mak b/src/posix.mak index 2d217c086bd7..703ef4a9b7e0 100644 --- a/src/posix.mak +++ b/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 \ diff --git a/src/win32.mak b/src/win32.mak index b3297bb1f014..d8f92cfd69ec 100644 --- a/src/win32.mak +++ b/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 \ diff --git a/test/fail_compilation/fail_pretty_errors.d b/test/fail_compilation/fail_pretty_errors.d new file mode 100644 index 000000000000..caff5a8e7ccc --- /dev/null +++ b/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; +}