From 4787d4b44955c362e4ee90c9d2931b1a6b28ba68 Mon Sep 17 00:00:00 2001 From: Sebastian Wilzbach Date: Wed, 11 Jul 2018 06:38:05 +0200 Subject: [PATCH 1/2] 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 | 127 +++++++++++++++++++++ src/dmd/frontend.d | 2 + src/dmd/globals.d | 1 + src/dmd/mars.d | 6 + src/dmd/root/stringtable.d | 14 +++ src/posix.mak | 2 +- src/win32.mak | 2 +- test/fail_compilation/fail_pretty_errors.d | 36 ++++++ 12 files changed, 237 insertions(+), 2 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 ee0479cbb4b2..a0a493b2307e 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 07769447dd69..86da9f5a5cbf 100644 --- a/src/dmd/cli.d +++ b/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" ), diff --git a/src/dmd/errors.d b/src/dmd/errors.d index c6ef54f38d5a..ed6d55f9db56 100644 --- a/src/dmd/errors.d +++ b/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 } diff --git a/src/dmd/filecache.d b/src/dmd/filecache.d new file mode 100644 index 000000000000..4d1ef1b49498 --- /dev/null +++ b/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(); + } +} diff --git a/src/dmd/frontend.d b/src/dmd/frontend.d index a990bb443c3a..f0a702ae3458 100644 --- a/src/dmd/frontend.d +++ b/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(); } /** diff --git a/src/dmd/globals.d b/src/dmd/globals.d index fc71c0fb998a..80da742b2e6e 100644 --- a/src/dmd/globals.d +++ b/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 diff --git a/src/dmd/mars.d b/src/dmd/mars.d index 711b2f37bda8..5399607a7f5b 100644 --- a/src/dmd/mars.d +++ b/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; } diff --git a/src/dmd/root/stringtable.d b/src/dmd/root/stringtable.d index 7ced8be5eef4..ef4dec53cd48 100644 --- a/src/dmd/root/stringtable.d +++ b/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) diff --git a/src/posix.mak b/src/posix.mak index e66280d4d916..978f255927b5 100644 --- a/src/posix.mak +++ b/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)) diff --git a/src/win32.mak b/src/win32.mak index cf3110dfbb23..91d90457ecd4 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..d25e8f77e050 --- /dev/null +++ b/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; +} From fc14e7803098ffcec9e3f70bfea537d37a6ce31c Mon Sep 17 00:00:00 2001 From: Sebastian Wilzbach Date: Wed, 19 Dec 2018 13:56:13 +0100 Subject: [PATCH 2/2] Don't print context errors if mixins are flushed to a file --- src/dmd/errors.d | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/dmd/errors.d b/src/dmd/errors.d index ed6d55f9db56..d4074827ae54 100644 --- a/src/dmd/errors.d +++ b/src/dmd/errors.d @@ -238,7 +238,8 @@ private void verrorPrint(const ref Loc loc, Color headerColor, const(char)* head // ignore invalid files loc != Loc.initial && // ignore mixins for now - !loc.filename.strstr(".d-mixin-")) + !loc.filename.strstr(".d-mixin-") && + !global.params.mixinOut) { import dmd.filecache : FileCache; auto fllines = FileCache.fileCache.addOrGetFile(loc.filename[0 .. strlen(loc.filename)]);