Better error messages - from reddit

Adam D. Ruppe destructionator at gmail.com
Tue Mar 5 14:08:12 UTC 2019


Improved patch. Error message:

/home/me/test/ooooo.d(21): 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)
/home/me/test/ooooo.d(21):        
isRandomAccessRange!(FilterResult!(unaryFun, int[]))/* was false 
*/



See that it now gives it as an errorSupplemental instead of 
random printf spam. My biggest problem with it now is 1) there's 
only one supplemental info pointer... so for multiple 
non-matching candidates, only the last one actually shows extra 
info! and lesser concern, 2) formatting in even more complex cases


Regardless, here's the code:


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..e9e5c1d 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;
      }

@@ -833,7 +834,7 @@ extern (C++) final class TemplateDeclaration 
: ScopeDsymbol
       *      dedtypes        deduced arguments
       * Return match level.
       */
-    extern (D) MATCH matchWithInstance(Scope* sc, 
TemplateInstance ti, Objects* dedtypes, Expressions* fargs, int 
flag)
+    extern (D) MatchWithSupplementalInformation 
matchWithInstance(Scope* sc, TemplateInstance ti, Objects* 
dedtypes, Expressions* fargs, int flag)
      {
          enum LOGM = 0;
          static if (LOGM)
@@ -848,11 +849,12 @@ extern (C++) final class 
TemplateDeclaration : ScopeDsymbol
          }
          MATCH m;
          size_t dedtypes_dim = dedtypes.dim;
+	StaticConditionResult supplementalInformation;

          dedtypes.zero();

          if (errors)
-            return MATCH.nomatch;
+            return 
MatchWithSupplementalInformation(MATCH.nomatch);

          size_t parameters_dim = parameters.dim;
          int variadic = isVariadic() !is null;
@@ -864,7 +866,7 @@ extern (C++) final class TemplateDeclaration 
: ScopeDsymbol
              {
                  printf(" no match: more arguments than 
parameters\n");
              }
-            return MATCH.nomatch;
+            return 
MatchWithSupplementalInformation(MATCH.nomatch);
          }

          assert(dedtypes_dim == parameters_dim);
@@ -970,7 +972,8 @@ extern (C++) final class TemplateDeclaration 
: ScopeDsymbol
              }

              // TODO: dedtypes => ti.tiargs ?
-            if (!evaluateConstraint(ti, sc, paramscope, 
dedtypes, fd))
+	    supplementalInformation = evaluateConstraint(ti, sc, 
paramscope, dedtypes, fd);
+            if (!supplementalInformation.result)
                  goto Lnomatch;
          }

@@ -1016,7 +1019,7 @@ extern (C++) final class 
TemplateDeclaration : ScopeDsymbol
          {
              printf("-TemplateDeclaration.matchWithInstance(this 
= %p, ti = %p) = %d\n", this, ti, m);
          }
-        return m;
+        return MatchWithSupplementalInformation(m);
      }

      /********************************************
@@ -1025,7 +1028,7 @@ extern (C++) final class 
TemplateDeclaration : ScopeDsymbol
       *      match   this is at least as specialized as td2
       *      0       td2 is more specialized than this
       */
-    MATCH leastAsSpecialized(Scope* sc, TemplateDeclaration td2, 
Expressions* fargs)
+    MatchWithSupplementalInformation leastAsSpecialized(Scope* 
sc, TemplateDeclaration td2, Expressions* fargs)
      {
          enum LOG_LEASTAS = 0;
          static if (LOG_LEASTAS)
@@ -1061,8 +1064,8 @@ extern (C++) final class 
TemplateDeclaration : ScopeDsymbol
          Objects dedtypes = Objects(td2.parameters.dim);

          // Attempt a type deduction
-        MATCH m = td2.matchWithInstance(sc, ti, &dedtypes, 
fargs, 1);
-        if (m > MATCH.nomatch)
+        auto m = td2.matchWithInstance(sc, ti, &dedtypes, fargs, 
1);
+        if (m.match > MATCH.nomatch)
          {
              /* A non-variadic template is more specialized than a
               * variadic one.
@@ -1082,7 +1085,12 @@ extern (C++) final class 
TemplateDeclaration : ScopeDsymbol
          {
              printf("  doesn't match, so is not as 
specialized\n");
          }
-        return MATCH.nomatch;
+        return MatchWithSupplementalInformation(MATCH.nomatch);
+    }
+
+    struct MatchWithSupplementalInformation {
+	MATCH match;
+	StaticConditionResult supplementalInformation;
      }

      /*************************************************
@@ -1101,13 +1109,14 @@ extern (C++) final class 
TemplateDeclaration : ScopeDsymbol
       *          bit 0-3     Match template parameters by 
inferred template arguments
       *          bit 4-7     Match template parameters by initial 
template arguments
       */
-    extern (D) MATCH 
deduceFunctionTemplateMatch(TemplateInstance ti, Scope* sc, ref 
FuncDeclaration fd, Type tthis, Expressions* fargs)
+    extern (D) MatchWithSupplementalInformation 
deduceFunctionTemplateMatch(TemplateInstance ti, Scope* sc, ref 
FuncDeclaration fd, Type tthis, Expressions* fargs)
      {
          size_t nfparams;
          size_t nfargs;
          size_t ntargs; // array size of tiargs
          size_t fptupindex = IDX_NOTFOUND;
          MATCH match = MATCH.exact;
+	StaticConditionResult supplementalInformation;
          MATCH matchTiargs = MATCH.exact;
          ParameterList fparameters; // function parameter list
          VarArg fvarargs; // function varargs
@@ -1142,7 +1151,7 @@ extern (C++) final class 
TemplateDeclaration : ScopeDsymbol
          dedtypes.zero();

          if (errors || fd.errors)
-            return MATCH.nomatch;
+            return 
MatchWithSupplementalInformation(MATCH.nomatch);

          // Set up scope for parameters
          Scope* paramscope = scopeForTemplateParameters(ti,sc);
@@ -2003,8 +2012,10 @@ extern (C++) final class 
TemplateDeclaration : ScopeDsymbol

          if (constraint)
          {
-            if (!evaluateConstraint(ti, sc, paramscope, dedargs, 
fd))
+	    supplementalInformation = evaluateConstraint(ti, sc, 
paramscope, dedargs, fd);
+            if (!supplementalInformation.result) {
                  goto Lnomatch;
+            }
          }

          version (none)
@@ -2018,18 +2029,19 @@ extern (C++) final class 
TemplateDeclaration : ScopeDsymbol

          paramscope.pop();
          //printf("\tmatch %d\n", match);
-        return cast(MATCH)(match | (matchTiargs << 4));
+        return 
MatchWithSupplementalInformation(cast(MATCH)(match | (matchTiargs 
<< 4)));

      Lnomatch:
          paramscope.pop();
          //printf("\tnomatch\n");
-        return MATCH.nomatch;
+	//asm { int 3; }
+        return MatchWithSupplementalInformation(MATCH.nomatch, 
supplementalInformation);

      Lerror:
          // todo: for the future improvement
          paramscope.pop();
          //printf("\terror\n");
-        return MATCH.nomatch;
+        return MatchWithSupplementalInformation(MATCH.nomatch);
      }

      /**************************************************
@@ -2592,10 +2604,14 @@ void functionResolve(Match* m, Dsymbol 
dstart, Loc loc, Scope* sc, Objects* tiar
              auto ti = new TemplateInstance(loc, td, tiargs);
              Objects dedtypes = Objects(td.parameters.dim);
              assert(td.semanticRun != PASS.init);
-            MATCH mta = td.matchWithInstance(sc, ti, &dedtypes, 
fargs, 0);
+            auto mta = td.matchWithInstance(sc, ti, &dedtypes, 
fargs, 0);
              //printf("matchWithInstance = %d\n", mta);
-            if (mta <= MATCH.nomatch || mta < ta_last)   // no 
match or less match
+            if (mta.match <= MATCH.nomatch || mta.match < 
ta_last)   // no match or less match
+            {
+	        if(pMessage && mta.supplementalInformation)
+		  *pMessage = mta.supplementalInformation.toChars();
                  return 0;
+            }

              ti.templateInstanceSemantic(sc, fargs);
              if (!ti.inst)               // if template failed to 
expand
@@ -2665,8 +2681,8 @@ void functionResolve(Match* m, Dsymbol 
dstart, Loc loc, Scope* sc, Objects* tiar
              if (mfa < m.last)
                  return 0;

-            if (mta < ta_last) goto Ltd_best2;
-            if (mta > ta_last) goto Ltd2;
+            if (mta.match < ta_last) goto Ltd_best2;
+            if (mta.match > ta_last) goto Ltd2;

              if (mfa < m.last) goto Ltd_best2;
              if (mfa > m.last) goto Ltd2;
@@ -2686,7 +2702,7 @@ void functionResolve(Match* m, Dsymbol 
dstart, Loc loc, Scope* sc, Objects* tiar
              td_best = td;
              ti_best = null;
              property = 0;   // (backward compatibility)
-            ta_last = mta;
+            ta_last = mta.match;
              m.last = mfa;
              m.lastf = fd;
              tthis_best = tthis_fd;
@@ -2708,12 +2724,18 @@ void functionResolve(Match* m, Dsymbol 
dstart, Loc loc, Scope* sc, Objects* tiar
              ti.parent = td.parent;  // Maybe calculating valid 
'enclosing' is unnecessary.

              auto fd = f;
-            int x = td.deduceFunctionTemplateMatch(ti, sc, fd, 
tthis, fargs);
+            auto information = 
td.deduceFunctionTemplateMatch(ti, sc, fd, tthis, fargs);
+	    int x = information.match;
              MATCH mta = cast(MATCH)(x >> 4);
              MATCH mfa = cast(MATCH)(x & 0xF);
              //printf("match:t/f = %d/%d\n", mta, mfa);
              if (!fd || mfa == MATCH.nomatch)
+	    {
+	
+	        if(pMessage && information.supplementalInformation)
+		  *pMessage = information.supplementalInformation.toChars();
                  continue;
+            }

              Type tthis_fd = fd.needThis() ? tthis : null;

@@ -2743,8 +2765,8 @@ void functionResolve(Match* m, Dsymbol 
dstart, Loc loc, Scope* sc, Objects* tiar
              if (td_best)
              {
                  // Disambiguate by picking the most specialized 
TemplateDeclaration
-                MATCH c1 = td.leastAsSpecialized(sc, td_best, 
fargs);
-                MATCH c2 = td_best.leastAsSpecialized(sc, td, 
fargs);
+                MATCH c1 = td.leastAsSpecialized(sc, td_best, 
fargs).match;
+                MATCH c2 = td_best.leastAsSpecialized(sc, td, 
fargs).match;
                  //printf("1: c1 = %d, c2 = %d\n", c1, c2);
                  if (c1 > c2) goto Ltd;
                  if (c1 < c2) goto Ltd_best;
@@ -6897,7 +6919,7 @@ extern (C++) class TemplateInstance : 
ScopeDsymbol
              assert(tempdecl._scope);
              // Deduce tdtypes
              tdtypes.setDim(tempdecl.parameters.dim);
-            if (!tempdecl.matchWithInstance(sc, this, &tdtypes, 
fargs, 2))
+            if (!tempdecl.matchWithInstance(sc, this, &tdtypes, 
fargs, 2).match)
              {
                  error("incompatible arguments for template 
instantiation");
                  return false;
@@ -6952,7 +6974,7 @@ extern (C++) class TemplateInstance : 
ScopeDsymbol
                  dedtypes.zero();
                  assert(td.semanticRun != PASS.init);

-                MATCH m = td.matchWithInstance(sc, this, 
&dedtypes, fargs, 0);
+                MATCH m = td.matchWithInstance(sc, this, 
&dedtypes, fargs, 0).match;
                  //printf("matchWithInstance = %d\n", m);
                  if (m <= MATCH.nomatch) // no match at all
                      return 0;
@@ -6961,8 +6983,8 @@ extern (C++) class TemplateInstance : 
ScopeDsymbol

                  // Disambiguate by picking the most specialized 
TemplateDeclaration
                  {
-                MATCH c1 = td.leastAsSpecialized(sc, td_best, 
fargs);
-                MATCH c2 = td_best.leastAsSpecialized(sc, td, 
fargs);
+                MATCH c1 = td.leastAsSpecialized(sc, td_best, 
fargs).match;
+                MATCH c2 = td_best.leastAsSpecialized(sc, td, 
fargs).match;
                  //printf("c1 = %d, c2 = %d\n", c1, c2);
                  if (c1 > c2) goto Ltd;
                  if (c1 < c2) goto Ltd_best;
@@ -7182,7 +7204,7 @@ extern (C++) class TemplateInstance : 
ScopeDsymbol
                              return 1;
                          }
                      }
-                    MATCH m = td.matchWithInstance(sc, this, 
&dedtypes, null, 0);
+                    MATCH m = td.matchWithInstance(sc, this, 
&dedtypes, null, 0).match;
                      if (m <= MATCH.nomatch)
                          return 0;
                  }
diff --git a/src/dmd/func.d b/src/dmd/func.d
index 4fce63a..ade0e95 100644
--- a/src/dmd/func.d
+++ b/src/dmd/func.d
@@ -2718,7 +2718,8 @@ FuncDeclaration resolveFuncCall(const ref 
Loc loc, Scope* sc, Dsymbol s,

      Match m;
      m.last = MATCH.nomatch;
-    functionResolve(&m, s, loc, sc, tiargs, tthis, fargs, null);
+    const(char)* supplementalInformation;
+    functionResolve(&m, s, loc, sc, tiargs, tthis, fargs, 
&supplementalInformation);
      auto orig_s = s;

      if (m.last > MATCH.nomatch && m.lastf)
@@ -2777,6 +2778,9 @@ FuncDeclaration resolveFuncCall(const ref 
Loc loc, Scope* sc, Dsymbol s,
                  tiargsBuf.peekString(), fargsBuf.peekString());

              printCandidates(loc, td);
+
+	    if(supplementalInformation)
+	    	.errorSupplemental(loc, "%s", supplementalInformation);
          }
          else if (od)
          {
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..0a05dc0 100644
--- a/src/dmd/staticcond.d
+++ b/src/dmd/staticcond.d
@@ -27,6 +27,59 @@ 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) {
+			auto p = e1.toString();
+			if(p.length) {
+				if(s.length)
+					s ~= "\n";
+				s ~= p;
+			}
+		}
+
+		if(e2) {
+			auto p = e2.toString();
+			if(p.length) {
+				if(s.length)
+					s ~= "\n";
+				s ~= p;
+			}
+		}
+
+		if(!e1 && !e2) {
+			if(wasEvaluated && !result) {
+				auto c = exp.toChars();
+
+				s ~= c[0 .. strlen(c)];
+				s ~= "/* was false */";
+			}
+		}
+
+		return s;
+	}
+}

  /********************************************
   * Semantically analyze and then evaluate a static condition at 
compile time.
@@ -42,37 +95,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 +144,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 +157,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 +166,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