Clarify GN nonfatal unused build arg messages.

Changes the messaging for nonfatal "unused build arg" messages to be
"WARNING" instead of "ERROR" when they are nonfatal and clarify that
the build has continued. Previously it would print "ERROR" and
continue anyway which was confusing.

Add an Err object move constructor and de-inline the assignment
operator (previously implicit).

Bug: 500696
Change-Id: Id6056b7a4d33466485f9e138a890e211f447817f
Reviewed-on: https://chromium-review.googlesource.com/762086
Commit-Queue: Brett Wilson <brettw@chromium.org>
Reviewed-by: Will Harris <wfh@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#516320}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: e953738ca60cdd39b2602ccbddba55b836b8e57e

tools/gn/args.cc

diff --git a/tools/gn/args.cc b/tools/gn/args.cc
index 967a65e..bd0db35 100644
--- a/tools/gn/args.cc
+++ b/tools/gn/args.cc
@@ -248,7 +248,7 @@
   std::string err_help(
       "The variable \"" + name + "\" was set as a build argument\n"
       "but never appeared in a declare_args() block in any buildfile.\n\n"
-      "To view all possible args, run \"gn args --list <builddir>\"");
+      "To view all possible args, run \"gn args --list <out_dir>\"");
 
   // Use all declare_args for a spelling suggestion.
   std::vector<base::StringPiece> candidates;

tools/gn/err.cc

diff --git a/tools/gn/err.cc b/tools/gn/err.cc
index 56e93bd..362bfb0 100644
--- a/tools/gn/err.cc
+++ b/tools/gn/err.cc
@@ -150,18 +150,26 @@
 }
 
 void Err::PrintToStdout() const {
-  InternalPrintToStdout(false);
+  InternalPrintToStdout(false, true);
+}
+
+void Err::PrintNonfatalToStdout() const {
+  InternalPrintToStdout(false, false);
 }
 
 void Err::AppendSubErr(const Err& err) {
   sub_errs_.push_back(err);
 }
 
-void Err::InternalPrintToStdout(bool is_sub_err) const {
+void Err::InternalPrintToStdout(bool is_sub_err, bool is_fatal) const {
   DCHECK(has_error_);
 
-  if (!is_sub_err)
-    OutputString("ERROR ", DECORATION_RED);
+  if (!is_sub_err) {
+    if (is_fatal)
+      OutputString("ERROR ", DECORATION_RED);
+    else
+      OutputString("WARNING ", DECORATION_RED);
+  }
 
   // File name and location.
   const InputFile* input_file = location_.file();
@@ -191,5 +199,5 @@
 
   // Sub errors.
   for (const auto& sub_err : sub_errs_)
-    sub_err.InternalPrintToStdout(true);
+    sub_err.InternalPrintToStdout(true, is_fatal);
 }

tools/gn/err.h

diff --git a/tools/gn/err.h b/tools/gn/err.h
index eeec31a..90f3150 100644
--- a/tools/gn/err.h
+++ b/tools/gn/err.h
@@ -24,7 +24,7 @@
 // below. They can provide additional context.
 class Err {
  public:
-  typedef std::vector<LocationRange> RangeList;
+  using RangeList = std::vector<LocationRange>;
 
   // Indicates no error.
   Err();
@@ -70,8 +70,20 @@
 
   void PrintToStdout() const;
 
+  // Prints to standard out but uses a "WARNING" messaging instead of the
+  // normal "ERROR" messaging. This is a property of the printing system rather
+  // than of the Err class because there is no expectation that code calling a
+  // function that take an Err check that the error is nonfatal and continue.
+  // Generally all Err objects with has_error() set are fatal.
+  //
+  // In some very specific cases code will detect a condition and print a
+  // nonfatal error to the screen instead of returning it. In these cases, that
+  // code can decide at printing time whether it will continue (and use this
+  // method) or not (and use PrintToStdout()).
+  void PrintNonfatalToStdout() const;
+
  private:
-  void InternalPrintToStdout(bool is_sub_err) const;
+  void InternalPrintToStdout(bool is_sub_err, bool is_fatal) const;
 
   bool has_error_;
   Location location_;

tools/gn/setup.cc

diff --git a/tools/gn/setup.cc b/tools/gn/setup.cc
index a448d92..e3b01a4 100644
--- a/tools/gn/setup.cc
+++ b/tools/gn/setup.cc
@@ -371,13 +371,14 @@
   }
 
   if (!build_settings_.build_args().VerifyAllOverridesUsed(&err)) {
-    // TODO(brettw) implement a system to have a different marker for
-    // warnings. Until we have a better system, print the error but don't
-    // return failure unless requested on the command line.
-    err.PrintToStdout();
     if (base::CommandLine::ForCurrentProcess()->HasSwitch(
-            switches::kFailOnUnusedArgs))
+            switches::kFailOnUnusedArgs)) {
+      err.PrintToStdout();
       return false;
+    }
+    err.PrintNonfatalToStdout();
+    OutputString("\nThe build continued as if that argument was "
+                 "unspecified.\n\n");
     return true;
   }