Better error messages - from reddit
Adam D. Ruppe
destructionator at gmail.com
Tue Mar 5 03:27:06 UTC 2019
On Tuesday, 5 March 2019 at 01:49:26 UTC, Nicholas Wilson wrote:
> Unfortunately both options are going to be nasty to implement
> because of the way that the compiler works, namely that you
> need to reevaluate the expressions and print their source.
Oh, I'm not so sure it is that hard. Well, it took me 90 mins
instead of the 30 I promised my wife, but the following patch
kinda sorta does it, a proof of concept at least. Not well, this
is by no means done, but it makes real progress on the status quo
even in its incomplete state.
Given the test program:
---
import std.algorithm;
void main() {
sort([1,2,3].filter!("a == 1"));
}
---
I get this output:
* * * * *
<snip some irrelevant spam from the top>
E1: E1: E1: E1: E1: cast(SwapStrategy)0 == cast(SwapStrategy)0
E2: E1: hasSwappableElements!(FilterResult!(unaryFun, int[]))
E2: isRandomAccessRange!(FilterResult!(unaryFun, int[]))/* was
false */
/home/me/test/ooooo.d(16): Error: template
std.algorithm.sorting.sort cannot deduce function from argument
types !()(FilterResult!(unaryFun, int[])), candidates are:
../generated/linux/release/64/../../../../../phobos/std/algorithm/sorting.d(1855): std.algorithm.sorting.sort(alias
ess = "a < b", SwapStrategy ss = SwapStrategy.unstable,
Range)(Range r) if ((ss == SwapStrategy.unstable &&
(hasSwappableElements!Range || hasAssignableElements!Range) || ss
!= SwapStrategy.unstable && hasAssignableElements!Range) &&
isRandomAccessRange!Range && hasSlicing!Range && hasLength!Range)
* * * * *
It told me what was going on! The last line of the spam is quite
valuable: it told me the isRandomAccessRange constraint failed...
and it even told me it was passed a FilterResult!(), which helps
more than you'd think when dealing with all kinds of hidden auto
returns and stuff.
That's some real world code right here - people ask about
basically that error every other week on this forum - and this
error is a lot more legible.
I'm telling you, this is doable.
Patch against dmd master follows:
diff --git a/src/dmd/cond.d b/src/dmd/cond.d
index 793cefc..ece927b 100644
--- a/src/dmd/cond.d
+++ b/src/dmd/cond.d
@@ -855,7 +855,7 @@ extern (C++) final class StaticIfCondition :
Condition
import dmd.staticcond;
bool errors;
- bool result = evalStaticCondition(sc, exp, exp,
errors);
+ bool result = evalStaticCondition(sc, exp, exp,
errors).result;
// Prevent repeated condition evaluation.
// See: fail_compilation/fail7815.d
diff --git a/src/dmd/dtemplate.d b/src/dmd/dtemplate.d
index 933df8d..b082dc3 100644
--- a/src/dmd/dtemplate.d
+++ b/src/dmd/dtemplate.d
@@ -684,10 +684,11 @@ extern (C++) final class
TemplateDeclaration : ScopeDsymbol
return protection;
}
+ import dmd.staticcond;
/****************************
* Check to see if constraint is satisfied.
*/
- extern (D) bool evaluateConstraint(TemplateInstance ti,
Scope* sc, Scope* paramscope, Objects* dedargs, FuncDeclaration
fd)
+ extern (D) StaticConditionResult
evaluateConstraint(TemplateInstance ti, Scope* sc, Scope*
paramscope, Objects* dedargs, FuncDeclaration fd)
{
/* Detect recursive attempts to instantiate this
template declaration,
* https://issues.dlang.org/show_bug.cgi?id=4072
@@ -713,7 +714,7 @@ extern (C++) final class TemplateDeclaration
: ScopeDsymbol
for (Scope* scx = sc; scx; scx = scx.enclosing)
{
if (scx == p.sc)
- return false;
+ return new StaticConditionResult(null,
false, false);
}
}
/* BUG: should also check for ref param differences
@@ -788,13 +789,13 @@ extern (C++) final class
TemplateDeclaration : ScopeDsymbol
ti.inst = ti; // temporary instantiation to enable
genIdent()
scx.flags |= SCOPE.constraint;
bool errors;
- bool result = evalStaticCondition(scx, constraint, e,
errors);
+ auto result = evalStaticCondition(scx, constraint, e,
errors);
ti.inst = null;
ti.symtab = null;
scx = scx.pop();
previous = pr.prev; // unlink from threaded list
if (errors)
- return false;
+ return new StaticConditionResult(null, false, false);
return result;
}
@@ -970,7 +971,7 @@ extern (C++) final class TemplateDeclaration
: ScopeDsymbol
}
// TODO: dedtypes => ti.tiargs ?
- if (!evaluateConstraint(ti, sc, paramscope,
dedtypes, fd))
+ if (!evaluateConstraint(ti, sc, paramscope,
dedtypes, fd).result)
goto Lnomatch;
}
@@ -2003,8 +2004,11 @@ extern (C++) final class
TemplateDeclaration : ScopeDsymbol
if (constraint)
{
- if (!evaluateConstraint(ti, sc, paramscope, dedargs,
fd))
+ auto result = evaluateConstraint(ti, sc, paramscope,
dedargs, fd);
+ if (!result.result) {
+ printf("%s\n", result.toChars());
goto Lnomatch;
+ }
}
version (none)
diff --git a/src/dmd/semantic2.d b/src/dmd/semantic2.d
index 36aced3..92bab02 100644
--- a/src/dmd/semantic2.d
+++ b/src/dmd/semantic2.d
@@ -101,7 +101,7 @@ private extern(C++) final class
Semantic2Visitor : Visitor
import dmd.staticcond;
bool errors;
- bool result = evalStaticCondition(sc, sa.exp, sa.exp,
errors);
+ bool result = evalStaticCondition(sc, sa.exp, sa.exp,
errors).result;
sc = sc.pop();
if (errors)
{
diff --git a/src/dmd/staticcond.d b/src/dmd/staticcond.d
index 5521a37..acc6298 100644
--- a/src/dmd/staticcond.d
+++ b/src/dmd/staticcond.d
@@ -27,6 +27,48 @@ import dmd.tokens;
import dmd.utils;
+class StaticConditionResult {
+ this(Expression exp, bool result, bool wasEvaluated) {
+ this.exp = exp;
+ this.result = result;
+ this.wasEvaluated = wasEvaluated;
+ }
+
+ Expression exp; /// original expression, for error messages
+ bool result; /// final result
+ bool wasEvaluated; /// if this part was evaluated at all
+
+ StaticConditionResult e1; /// result on the one side, if done
+ StaticConditionResult e2; /// result on the other side, if done
+
+ const(char)* toChars() {
+ string s = this.toString();
+ s ~= "\n";
+ return s.ptr;
+ }
+
+ override string toString() {
+ import core.stdc.string;
+ string s;
+ if(e1) {
+ s ~= "E1: " ~ e1.toString() ~ "\n";
+ }
+
+ if(e2) {
+ s ~= "E2: " ~ e2.toString() ~ "\n";
+ }
+
+ if(!e1 && !e2) {
+ auto c = exp.toChars();
+
+ s ~= c[0 .. strlen(c)];
+ if(wasEvaluated && !result)
+ s ~= "/* was false */";
+ }
+
+ return s;
+ }
+}
/********************************************
* Semantically analyze and then evaluate a static condition at
compile time.
@@ -42,37 +84,48 @@ import dmd.utils;
* true if evaluates to true
*/
-bool evalStaticCondition(Scope* sc, Expression exp, Expression
e, ref bool errors)
+StaticConditionResult evalStaticCondition(Scope* sc, Expression
exp, Expression e, ref bool errors)
{
+//if(e.parens) printf("PARANS %s\n\n", e.toChars);
if (e.op == TOK.andAnd || e.op == TOK.orOr)
{
LogicalExp aae = cast(LogicalExp)e;
- bool result = evalStaticCondition(sc, exp, aae.e1,
errors);
+ auto result = new StaticConditionResult(e, false, true);
+ auto r2 = evalStaticCondition(sc, exp, aae.e1, errors);
+ result.e1 = r2;
+ result.result = r2.result;
if (errors)
- return false;
+ return new StaticConditionResult(exp, false, false);
if (e.op == TOK.andAnd)
{
- if (!result)
- return false;
+ if (!result.result)
+ return result;
}
else
{
- if (result)
- return true;
+ if (result.result)
+ return result;
}
- result = evalStaticCondition(sc, exp, aae.e2, errors);
- return !errors && result;
+ auto r3 = evalStaticCondition(sc, exp, aae.e2, errors);
+ result.e2 = r3;
+ result.result = r3.result;
+ if(errors)
+ result.result = false;
+ return result; // !errors && result;
}
if (e.op == TOK.question)
{
CondExp ce = cast(CondExp)e;
- bool result = evalStaticCondition(sc, exp, ce.econd,
errors);
+ auto result = evalStaticCondition(sc, exp, ce.econd,
errors);
if (errors)
- return false;
- Expression leg = result ? ce.e1 : ce.e2;
- result = evalStaticCondition(sc, exp, leg, errors);
- return !errors && result;
+ return new StaticConditionResult(exp, false, false);
+ Expression leg = result.result ? ce.e1 : ce.e2;
+ result.e1 = evalStaticCondition(sc, exp, leg, errors);
+ result.result = result.e1.result;
+ if(errors)
+ result.result = false;
+ return result;
}
uint nerrors = global.errors;
@@ -80,6 +133,8 @@ bool evalStaticCondition(Scope* sc, Expression
exp, Expression e, ref bool error
sc = sc.startCTFE();
sc.flags |= SCOPE.condition;
+ auto originalE = e;
+
e = e.expressionSemantic(sc);
e = resolveProperties(sc, e);
@@ -91,7 +146,7 @@ bool evalStaticCondition(Scope* sc, Expression
exp, Expression e, ref bool error
e.type.toBasetype() == Type.terror)
{
errors = true;
- return false;
+ return new StaticConditionResult(exp, false, false);
}
e = resolveAliasThis(sc, e);
@@ -100,17 +155,17 @@ bool evalStaticCondition(Scope* sc,
Expression exp, Expression e, ref bool error
{
exp.error("expression `%s` of type `%s` does not have a
boolean value", exp.toChars(), e.type.toChars());
errors = true;
- return false;
+ return new StaticConditionResult(exp, false, false);
}
e = e.ctfeInterpret();
if (e.isBool(true))
- return true;
+ return new StaticConditionResult(originalE, true, true);
else if (e.isBool(false))
- return false;
+ return new StaticConditionResult(originalE, false, true);
e.error("expression `%s` is not constant", e.toChars());
errors = true;
- return false;
+ return new StaticConditionResult(exp, false, false);
}
More information about the Digitalmars-d
mailing list