Improve error formatting

Changes:

* The divider lines are gone. These were in practice a bit confusing,
  in particular with --show-trace or --keep-going, since then there
  were multiple lines, suggesting a start/end which wasn't the case.

* Instead, multi-line error messages are now indented to align with
  the prefix (e.g. "error: ").

* The 'description' field is gone since we weren't really using it.

* 'hint' is renamed to 'msg' since it really wasn't a hint.

* The error is now printed *before* the location info.

* The 'name' field is no longer printed since most of the time it
  wasn't very useful since it was just the name of the exception (like
  EvalError). Ideally in the future this would be a unique, easily
  googleable error ID (like rustc).

* "trace:" is now just "…". This assumes error contexts start with
  something like "while doing X".

Example before:

  error: --- AssertionError ---------------------------------------------------------------------------------------- nix
  at: (7:7) in file: /home/eelco/Dev/nixpkgs/pkgs/applications/misc/hello/default.nix

       6|
       7|   x = assert false; 1;
        |       ^
       8|

  assertion 'false' failed
  ----------------------------------------------------- show-trace -----------------------------------------------------
  trace: while evaluating the attribute 'x' of the derivation 'hello-2.10'
  at: (192:11) in file: /home/eelco/Dev/nixpkgs/pkgs/stdenv/generic/make-derivation.nix

     191|         // (lib.optionalAttrs (!(attrs ? name) && attrs ? pname && attrs ? version)) {
     192|           name = "${attrs.pname}-${attrs.version}";
        |           ^
     193|         } // (lib.optionalAttrs (stdenv.hostPlatform != stdenv.buildPlatform && !dontAddHostSuffix && (attrs ? name || (attrs ? pname && attrs ? version)))) {

Example after:

  error: assertion 'false' failed

         at: (7:7) in file: /home/eelco/Dev/nixpkgs/pkgs/applications/misc/hello/default.nix

              6|
              7|   x = assert false; 1;
               |       ^
              8|

         … while evaluating the attribute 'x' of the derivation 'hello-2.10'

         at: (192:11) in file: /home/eelco/Dev/nixpkgs/pkgs/stdenv/generic/make-derivation.nix

            191|         // (lib.optionalAttrs (!(attrs ? name) && attrs ? pname && attrs ? version)) {
            192|           name = "${attrs.pname}-${attrs.version}";
               |           ^
            193|         } // (lib.optionalAttrs (stdenv.hostPlatform != stdenv.buildPlatform && !dontAddHostSuffix && (attrs ? name || (attrs ? pname && attrs ? version)))) {

src/build-remote/build-remote.cc

@@ -172,13 +172,14 @@ static int main_build_remote(int argc, char * * argv)
  else
  {
  // build the hint template.
- string hintstring = "derivation: %s\nrequired (system, features): (%s, %s)";
- hintstring += "\n%s available machines:";
- hintstring += "\n(systems, maxjobs, supportedFeatures, mandatoryFeatures)";
+ string errorText =
+ "Failed to find a machine for remote build!\n"
+ "derivation: %s\nrequired (system, features): (%s, %s)";
+ errorText += "\n%s available machines:";
+ errorText += "\n(systems, maxjobs, supportedFeatures, mandatoryFeatures)";
 
- for (unsigned int i = 0; i < machines.size(); ++i) {
- hintstring += "\n(%s, %s, %s, %s)";
- }
+ for (unsigned int i = 0; i < machines.size(); ++i)
+ errorText += "\n(%s, %s, %s, %s)";
 
  // add the template values.
  string drvstr;
@@ -187,25 +188,21 @@ static int main_build_remote(int argc, char * * argv)
  else
  drvstr = "<unknown>";
 
- auto hint = hintformat(hintstring);
- hint
- % drvstr
- % neededSystem
- % concatStringsSep<StringSet>(", ", requiredFeatures)
- % machines.size();
-
- for (auto & m : machines) {
- hint % concatStringsSep<vector<string>>(", ", m.systemTypes)
- % m.maxJobs
- % concatStringsSep<StringSet>(", ", m.supportedFeatures)
- % concatStringsSep<StringSet>(", ", m.mandatoryFeatures);
- }
+ auto error = hintformat(errorText);
+ error
+  % drvstr
+  % neededSystem
+  % concatStringsSep<StringSet>(", ", requiredFeatures)
+  % machines.size();
+
+ for (auto & m : machines)
+  error
+  % concatStringsSep<vector<string>>(", ", m.systemTypes)
+  % m.maxJobs
+  % concatStringsSep<StringSet>(", ", m.supportedFeatures)
+  % concatStringsSep<StringSet>(", ", m.mandatoryFeatures);
 
- logErrorInfo(canBuildLocally ? lvlChatty : lvlWarn, {
- .name = "Remote build",
- .description = "Failed to find a machine for remote build!",
- .hint = hint
- });
+ printMsg(canBuildLocally ? lvlChatty : lvlWarn, error);
 
  std::cerr << "# decline\n";
  }
@@ -230,12 +227,9 @@ static int main_build_remote(int argc, char * * argv)
 
  } catch (std::exception & e) {
  auto msg = chomp(drainFD(5, false));
- logError({
- .name = "Remote build",
- .hint = hintfmt("cannot build on '%s': %s%s",
- bestMachine->storeUri, e.what(),
- (msg.empty() ? "" : ": " + msg))
- });
+ printError("cannot build on '%s': %s%s",
+ bestMachine->storeUri, e.what(),
+ msg.empty() ? "" : ": " + msg);
  bestMachine->enabled = false;
  continue;
  }

src/libexpr/attr-set.hh

@@ -77,7 +77,7 @@ public:
  auto a = get(name);
  if (!a)
  throw Error({
- .hint = hintfmt("attribute '%s' missing", name),
+ .msg = hintfmt("attribute '%s' missing", name),
  .errPos = pos
  });
 

src/libexpr/eval-inline.hh

@@ -10,7 +10,7 @@ namespace nix {
 LocalNoInlineNoReturn(void throwEvalError(const Pos & pos, const char * s))
 {
  throw EvalError({
- .hint = hintfmt(s),
+ .msg = hintfmt(s),
  .errPos = pos
  });
 }
@@ -24,7 +24,7 @@ LocalNoInlineNoReturn(void throwTypeError(const char * s, const Value & v))
 LocalNoInlineNoReturn(void throwTypeError(const Pos & pos, const char * s, const Value & v))
 {
  throw TypeError({
- .hint = hintfmt(s, showType(v)),
+ .msg = hintfmt(s, showType(v)),
  .errPos = pos
  });
 }

src/libexpr/eval.cc

@@ -622,7 +622,7 @@ LocalNoInlineNoReturn(void throwEvalError(const char * s, const string & s2))
 LocalNoInlineNoReturn(void throwEvalError(const Pos & pos, const char * s, const string & s2))
 {
  throw EvalError({
- .hint = hintfmt(s, s2),
+ .msg = hintfmt(s, s2),
  .errPos = pos
  });
 }
@@ -635,7 +635,7 @@ LocalNoInlineNoReturn(void throwEvalError(const char * s, const string & s2, con
 LocalNoInlineNoReturn(void throwEvalError(const Pos & pos, const char * s, const string & s2, const string & s3))
 {
  throw EvalError({
- .hint = hintfmt(s, s2, s3),
+ .msg = hintfmt(s, s2, s3),
  .errPos = pos
  });
 }
@@ -644,47 +644,47 @@ LocalNoInlineNoReturn(void throwEvalError(const Pos & p1, const char * s, const
 {
  // p1 is where the error occurred; p2 is a position mentioned in the message.
  throw EvalError({
- .hint = hintfmt(s, sym, p2),
+ .msg = hintfmt(s, sym, p2),
  .errPos = p1
  });
 }
 
 LocalNoInlineNoReturn(void throwTypeError(const Pos & pos, const char * s))
 {
  throw TypeError({
- .hint = hintfmt(s),
+ .msg = hintfmt(s),
  .errPos = pos
  });
 }
 
 LocalNoInlineNoReturn(void throwTypeError(const Pos & pos, const char * s, const ExprLambda & fun, const Symbol & s2))
 {
  throw TypeError({
- .hint = hintfmt(s, fun.showNamePos(), s2),
+ .msg = hintfmt(s, fun.showNamePos(), s2),
  .errPos = pos
  });
 }
 
 LocalNoInlineNoReturn(void throwAssertionError(const Pos & pos, const char * s, const string & s1))
 {
  throw AssertionError({
- .hint = hintfmt(s, s1),
+ .msg = hintfmt(s, s1),
  .errPos = pos
  });
 }
 
 LocalNoInlineNoReturn(void throwUndefinedVarError(const Pos & pos, const char * s, const string & s1))
 {
  throw UndefinedVarError({
- .hint = hintfmt(s, s1),
+ .msg = hintfmt(s, s1),
  .errPos = pos
  });
 }
 
 LocalNoInlineNoReturn(void throwMissingArgumentError(const Pos & pos, const char * s, const string & s1))
 {
  throw MissingArgumentError({
- .hint = hintfmt(s, s1),
+ .msg = hintfmt(s, s1),
  .errPos = pos
  });
 }
@@ -2057,7 +2057,7 @@ void EvalState::printStats()
 string ExternalValueBase::coerceToString(const Pos & pos, PathSet & context, bool copyMore, bool copyToStore) const
 {
  throw TypeError({
- .hint = hintfmt("cannot coerce %1% to a string", showType()),
+ .msg = hintfmt("cannot coerce %1% to a string", showType()),
  .errPos = pos
  });
 }

src/libexpr/nixexpr.cc

@@ -284,7 +284,7 @@ void ExprVar::bindVars(const StaticEnv & env)
  "undefined variable" error now. */
  if (withLevel == -1)
  throw UndefinedVarError({
- .hint = hintfmt("undefined variable '%1%'", name),
+ .msg = hintfmt("undefined variable '%1%'", name),
  .errPos = pos
  });
  fromWith = true;

src/libexpr/nixexpr.hh

@@ -239,7 +239,7 @@ struct ExprLambda : Expr
  {
  if (!arg.empty() && formals && formals->argNames.find(arg) != formals->argNames.end())
  throw ParseError({
- .hint = hintfmt("duplicate formal function argument '%1%'", arg),
+ .msg = hintfmt("duplicate formal function argument '%1%'", arg),
  .errPos = pos
  });
  };

src/libexpr/parser.y

@@ -32,7 +32,7 @@ namespace nix {
  Path basePath;
  Symbol file;
  FileOrigin origin;
- ErrorInfo error;
+ std::optional<ErrorInfo> error;
  Symbol sLetBody;
  ParseData(EvalState & state)
  : state(state)
@@ -66,16 +66,16 @@ namespace nix {
 static void dupAttr(const AttrPath & attrPath, const Pos & pos, const Pos & prevPos)
 {
  throw ParseError({
- .hint = hintfmt("attribute '%1%' already defined at %2%",
- showAttrPath(attrPath), prevPos),
+ .msg = hintfmt("attribute '%1%' already defined at %2%",
+  showAttrPath(attrPath), prevPos),
  .errPos = pos
  });
 }
 
 static void dupAttr(Symbol attr, const Pos & pos, const Pos & prevPos)
 {
  throw ParseError({
- .hint = hintfmt("attribute '%1%' already defined at %2%", attr, prevPos),
+ .msg = hintfmt("attribute '%1%' already defined at %2%", attr, prevPos),
  .errPos = pos
  });
 }
@@ -146,7 +146,7 @@ static void addFormal(const Pos & pos, Formals * formals, const Formal & formal)
 {
  if (!formals->argNames.insert(formal.name).second)
  throw ParseError({
- .hint = hintfmt("duplicate formal function argument '%1%'",
+ .msg = hintfmt("duplicate formal function argument '%1%'",
  formal.name),
  .errPos = pos
  });
@@ -258,7 +258,7 @@ static inline Pos makeCurPos(const YYLTYPE & loc, ParseData * data)
 void yyerror(YYLTYPE * loc, yyscan_t scanner, ParseData * data, const char * error)
 {
  data->error = {
- .hint = hintfmt(error),
+ .msg = hintfmt(error),
  .errPos = makeCurPos(*loc, data)
  };
 }
@@ -338,7 +338,7 @@ expr_function
  | LET binds IN expr_function
  { if (!$2->dynamicAttrs.empty())
  throw ParseError({
- .hint = hintfmt("dynamic attributes not allowed in let"),
+ .msg = hintfmt("dynamic attributes not allowed in let"),
  .errPos = CUR_POS
  });
  $$ = new ExprLet($2, $4);
@@ -418,7 +418,7 @@ expr_simple
  static bool noURLLiterals = settings.isExperimentalFeatureEnabled("no-url-literals");
  if (noURLLiterals)
  throw ParseError({
- .hint = hintfmt("URL literals are disabled"),
+ .msg = hintfmt("URL literals are disabled"),
  .errPos = CUR_POS
  });
  $$ = new ExprString(data->symbols.create($1));
@@ -491,7 +491,7 @@ attrs
  delete str;
  } else
  throw ParseError({
- .hint = hintfmt("dynamic attributes not allowed in inherit"),
+ .msg = hintfmt("dynamic attributes not allowed in inherit"),
  .errPos = makeCurPos(@2, data)
  });
  }
@@ -576,7 +576,7 @@ Expr * EvalState::parse(const char * text, FileOrigin origin,
  ParseData data(*this);
  data.origin = origin;
  switch (origin) {
- case foFile: 
+ case foFile:
  data.file = data.symbols.create(path);
  break;
  case foStdin:
@@ -593,7 +593,7 @@ Expr * EvalState::parse(const char * text, FileOrigin origin,
  int res = yyparse(scanner, &data);
  yylex_destroy(scanner);
 
- if (res) throw ParseError(data.error);
+ if (res) throw ParseError(data.error.value());
 
  data.result->bindVars(staticEnv);
 
@@ -703,7 +703,7 @@ Path EvalState::findFile(SearchPath & searchPath, const string & path, const Pos
  return corepkgsPrefix + path.substr(4);
 
  throw ThrownError({
- .hint = hintfmt(evalSettings.pureEval
+ .msg = hintfmt(evalSettings.pureEval
  ? "cannot look up '<%s>' in pure evaluation mode (use '--impure' to override)"
  : "file '%s' was not found in the Nix search path (add it using $NIX_PATH or -I)",
  path),
@@ -725,8 +725,7 @@ std::pair<bool, std::string> EvalState::resolveSearchPathElem(const SearchPathEl
  store, resolveUri(elem.second), "source", false).first.storePath) };
  } catch (FileTransferError & e) {
  logWarning({
- .name = "Entry download",
- .hint = hintfmt("Nix search path entry '%1%' cannot be downloaded, ignoring", elem.second)
+ .msg = hintfmt("Nix search path entry '%1%' cannot be downloaded, ignoring", elem.second)
  });
  res = { false, "" };
  }
@@ -736,8 +735,7 @@ std::pair<bool, std::string> EvalState::resolveSearchPathElem(const SearchPathEl
  res = { true, path };
  else {
  logWarning({
- .name = "Entry not found",
- .hint = hintfmt("warning: Nix search path entry '%1%' does not exist, ignoring", elem.second)
+ .msg = hintfmt("warning: Nix search path entry '%1%' does not exist, ignoring", elem.second)
  });
  res = { false, "" };
  }