Require commas between items in a GN list.

tools/gn/input_file_manager.cc

 // Copyright (c) 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "tools/gn/input_file_manager.h"
 
 #include "base/bind.h"
 #include "base/stl_util.h"
 #include "tools/gn/filesystem_utils.h"
 #include "tools/gn/parser.h"
 #include "tools/gn/scheduler.h"
 #include "tools/gn/scope_per_file_provider.h"
 #include "tools/gn/tokenizer.h"
 #include "tools/gn/trace.h"
 
 namespace {
 
 void InvokeFileLoadCallback(const InputFileManager::FileLoadCallback& cb,
                             const ParseNode* node) {
   cb.Run(node);
 }
 
+bool DoLoadFile(const LocationRange& origin,
+                const BuildSettings* build_settings,
+                const SourceFile& name,
+                InputFile* file,
+                std::vector<Token>* tokens,
+                scoped_ptr<ParseNode>* root,
+                Err* err) {
+  // Do all of this stuff outside the lock. We should not give out file
+  // pointers until the read is complete.
+  if (g_scheduler->verbose_logging()) {
+    std::string logmsg = name.value();
+    if (origin.begin().file())
+      logmsg += " (referenced from " + origin.begin().Describe(false) + ")";
+    g_scheduler->Log("Loading", logmsg);
+  }
+
+  // Read.
+  base::FilePath primary_path = build_settings->GetFullPath(name);
+  ScopedTrace load_trace(TraceItem::TRACE_FILE_LOAD, name.value());
+  if (!file->Load(primary_path)) {
+    if (!build_settings->secondary_source_path().empty()) {
+      // Fall back to secondary source tree.
+      base::FilePath secondary_path =
+          build_settings->GetFullPathSecondary(name);
+      if (!file->Load(secondary_path)) {
+        *err = Err(origin, "Can't load input file.",
+                   "Unable to load either \n" +
+                   FilePathToUTF8(primary_path) + " or \n" +
+                   FilePathToUTF8(secondary_path));
+        return false;
+      }
+    } else {
+      *err = Err(origin,
+                 "Unable to load \"" + FilePathToUTF8(primary_path) + "\".");
+      return false;
+    }
+  }
+  load_trace.Done();
+
+  ScopedTrace exec_trace(TraceItem::TRACE_FILE_PARSE, name.value());
+
+  // Tokenize.
+  *tokens = Tokenizer::Tokenize(file, err);
+  if (err->has_error())
+    return false;
+
+  // Parse.
+  *root = Parser::Parse(*tokens, err);
+  if (err->has_error())
+    return false;
+
+  exec_trace.Done();
+  return true;
+}
+
 }  // namespace
 
 InputFileManager::InputFileData::InputFileData(const SourceFile& file_name)
     : file(file_name),
       loaded(false),
       sync_invocation(false) {
 }
 
 InputFileManager::InputFileData::~InputFileData() {
 }
@@ skipping 171 matching lines @@
   Err err;
   if (!LoadFile(origin, build_settings, name, file, &err))
     g_scheduler->FailWithError(err);
 }
 
 bool InputFileManager::LoadFile(const LocationRange& origin,
                                 const BuildSettings* build_settings,
                                 const SourceFile& name,
                                 InputFile* file,
                                 Err* err) {
-  // Do all of this stuff outside the lock. We should not give out file
-  // pointers until the read is complete.
-  if (g_scheduler->verbose_logging()) {
-    std::string logmsg = name.value();
-    if (origin.begin().file())
-      logmsg += " (referenced from " + origin.begin().Describe(false) + ")";
-    g_scheduler->Log("Loading", logmsg);
-  }
+  std::vector<Token> tokens;
+  scoped_ptr<ParseNode> root;
+  bool success = DoLoadFile(origin, build_settings, name, file,
+                            &tokens, &root, err);
+  // Can't return early. We have to ensure that the completion event is
+  // signaled in all cases bacause another thread could be blocked on this one.
 
-  // Read.
-  base::FilePath primary_path = build_settings->GetFullPath(name);
-  ScopedTrace load_trace(TraceItem::TRACE_FILE_LOAD, name.value());
-  if (!file->Load(primary_path)) {
-    if (!build_settings->secondary_source_path().empty()) {
-      // Fall back to secondary source tree.
-      base::FilePath secondary_path =
-          build_settings->GetFullPathSecondary(name);
-      if (!file->Load(secondary_path)) {
-        *err = Err(origin, "Can't load input file.",
-                   "Unable to load either \n" +
-                   FilePathToUTF8(primary_path) + " or \n" +
-                   FilePathToUTF8(secondary_path));
-        return false;
-      }
-    } else {
-      *err = Err(origin,
-                 "Unable to load \"" + FilePathToUTF8(primary_path) + "\".");
-      return false;
-    }
-  }
-  load_trace.Done();
-
-  ScopedTrace exec_trace(TraceItem::TRACE_FILE_PARSE, name.value());
-
-  // Tokenize.
-  std::vector<Token> tokens = Tokenizer::Tokenize(file, err);
-  if (err->has_error())
-    return false;
-
-  // Parse.
-  scoped_ptr<ParseNode> root = Parser::Parse(tokens, err);
-  if (err->has_error())
-    return false;
+  // Save this pointer for running the callbacks below, which happens after the
+  // scoped ptr ownership is taken away inside the lock.
   ParseNode* unowned_root = root.get();
 
-  exec_trace.Done();
-
   std::vector<FileLoadCallback> callbacks;
   {
     base::AutoLock lock(lock_);
     DCHECK(input_files_.find(name) != input_files_.end());
 
     InputFileData* data = input_files_[name];
     data->loaded = true;
-    data->tokens.swap(tokens);
-    data->parsed_root = root.Pass();
+    if (success) {
+      data->tokens.swap(tokens);
+      data->parsed_root = root.Pass();
+    }
 
     // Unblock waiters on this event.
     //
     // It's somewhat bad to signal this inside the lock. When it's used, it's
     // lazily created inside the lock. So we need to do the check and signal
     // inside the lock to avoid race conditions on the lazy creation of the
     // lock.
     //
     // We could avoid this by creating the lock every time, but the lock is
     // very seldom used and will generally be NULL, so my current theory is that
     // several signals of a completion event inside a lock is better than
     // creating about 1000 extra locks (one for each file).
     if (data->completion_event)
       data->completion_event->Signal();
 
     callbacks.swap(data->scheduled_callbacks);
   }
 
   // Run pending invocations. Theoretically we could schedule each of these
   // separately to get some parallelism. But normally there will only be one
   // item in the list, so that's extra overhead and complexity for no gain.
-  for (size_t i = 0; i < callbacks.size(); i++)
-    callbacks[i].Run(unowned_root);
-  return true;
+  if (success) {
+    for (size_t i = 0; i < callbacks.size(); i++)
+      callbacks[i].Run(unowned_root);
+  }
+  return success;
 }