GN: Fix CPU detection in Visual Studio generator

tools/gn/visual_studio_writer.cc

 // Copyright 2016 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/visual_studio_writer.h"
 
 #include <algorithm>
 #include <map>
 #include <set>
 #include <string>
@@ skipping 160 matching lines @@
     const std::string& _guid,
     const std::string& _label_dir_path,
     const std::string& _config_platform)
     : SolutionEntry(_name, _path, _guid),
       label_dir_path(_label_dir_path),
       config_platform(_config_platform) {}
 
 VisualStudioWriter::SolutionProject::~SolutionProject() = default;
 
 VisualStudioWriter::VisualStudioWriter(const BuildSettings* build_settings,
+                                       bool is_debug_config,
+                                       const char* config_platform,
                                        Version version)
     : build_settings_(build_settings),
+      is_debug_config_(is_debug_config),
+      config_platform_(config_platform),
       ninja_path_output_(build_settings->build_dir(),
                          build_settings->root_path_utf8(),
                          EscapingMode::ESCAPE_NINJA_COMMAND) {
-  const Value* value = build_settings->build_args().GetArgOverride("is_debug");
-  is_debug_config_ = value == nullptr || value->boolean_value();
-  config_platform_ = "Win32";
-  value = build_settings->build_args().GetArgOverride(variables::kTargetCpu);
-  if (value != nullptr && value->string_value() == "x64")
-    config_platform_ = "x64";
-
   switch (version) {
     case Version::Vs2013:
       project_version_ = kProjectVersionVs2013;
       toolset_version_ = kToolsetVersionVs2013;
       version_string_ = kVersionStringVs2013;
       break;
     case Version::Vs2015:
       project_version_ = kProjectVersionVs2015;
       toolset_version_ = kToolsetVersionVs2015;
       version_string_ = kVersionStringVs2015;
@@ skipping 10 matching lines @@
   STLDeleteContainerPointers(folders_.begin(), folders_.end());
 }
 
 // static
 bool VisualStudioWriter::RunAndWriteFiles(const BuildSettings* build_settings,
                                           Builder* builder,
                                           Version version,
                                           Err* err) {
   std::vector<const Target*> targets = builder->GetAllResolvedTargets();
 
-  VisualStudioWriter writer(build_settings, version);
+  bool is_debug_config = true;
+  const char* config_platform = "Win32";
+
+  // Assume the "is_debug" and "target_cpu" variables do not change
+  // between different toolchains.

[brettw, 2016/02/24 21:51:48]

This is not the case (CPU does change between toolchains, the PNaCl toolchain in
Chrome does this, and for a long time 32-bit Windows builds of Chrome were
actually cross-compiles using 32-bit target and 64-bit host (we changed this
since it's wasteful and unnecessary to compile those things more than once).

It's reasonable that the VS generator isn't perfect, especially if this ends up
not mattering too much in practice. Can you reword this comment to make more
clear that this is an incorrect simplifying assumption, and what the problems
are that will result?

[NGG, 2016/02/24 22:19:15]

Cross-compiles work with this if only the "current_*" variables change with the
toolchain and not the "target_*" ones, I think this is true.
If "target_cpu" really changes then here we would need to find the default
toolchain's scope and I didn't find an easier way than to look at targets in a
loop until we find one with the default toolchain. I didn't want that because it
would probably slow down the generator a bit.

[brettw, 2016/02/24 23:38:15]

Oh, I missed "target" and I thought you were talking about "current". You're
right.

+  if (!targets.empty()) {
+    const Scope* scope = targets.front()->settings()->base_config();
+    const Value* is_debug_value = scope->GetValue("is_debug");

[brettw, 2016/02/24 21:51:48]

"is_debug" is a variable in the Chrome build, not something built into GN. Can
we avoid making assumptions about such things here?

I realize you just moved existing code so this is partially a question for
tmoniuszko: What happens if we don't do this? Can we just have a "generic"
configuration that VS sees? I don't think there's any actual switching we can do
in the IDE.

[NGG, 2016/02/24 22:19:15]

+1 for this. It could be called "GN". I don't know of any VS plugins that use
the configuration name for anything.

[Tomasz Moniuszko, 2016/02/25 11:06:14]

Visual Studio allows to rename configuration to any name so I guess it should
work. I think it's convenient to have Debug/Release string in configuration name
(as I switch between debug/non-debug solutions often) but it's not critical.
Removing is_debug_config_ sgtm.

+    is_debug_config =
+        is_debug_value == nullptr || is_debug_value->boolean_value();
+    const Value* target_cpu_value = scope->GetValue(variables::kTargetCpu);
+    if (target_cpu_value != nullptr &&
+        target_cpu_value->string_value() == "x64")
+      config_platform = "x64";
+  }
+
+  VisualStudioWriter writer(build_settings, is_debug_config, config_platform,
+                            version);
   writer.projects_.reserve(targets.size());
   writer.folders_.reserve(targets.size());
 
   for (const Target* target : targets) {
     // Skip actions and groups.
     if (target->output_type() == Target::GROUP ||
         target->output_type() == Target::COPY_FILES ||
         target->output_type() == Target::ACTION ||
         target->output_type() == Target::ACTION_FOREACH) {
       continue;
@@ skipping 14 matching lines @@
             [](const SolutionEntry* a, const SolutionEntry* b) {
               return a->path < b->path;
             });
 
   writer.ResolveSolutionFolders();
   return writer.WriteSolutionFile(err);
 }
 
 bool VisualStudioWriter::WriteProjectFiles(const Target* target, Err* err) {
   std::string project_name = target->label().name();
-  std::string project_config_platform = config_platform_;
+  const char* project_config_platform = config_platform_;
   if (!target->settings()->is_default()) {
     project_name += "_" + target->toolchain()->label().name();
-    project_config_platform = target->toolchain()
-                                  ->settings()
-                                  ->build_settings()
-                                  ->build_args()
-                                  .GetArgOverride(variables::kCurrentCpu)
-                                  ->string_value();
-    if (project_config_platform == "x86")
+    const Value* value =
+        target->settings()->base_config()->GetValue(variables::kCurrentCpu);
+    if (value != nullptr && value->string_value() == "x64")
+      project_config_platform = "x64";
+    else
       project_config_platform = "Win32";
   }
 
   SourceFile target_file = GetTargetOutputDir(target).ResolveRelativeFile(
       Value(nullptr, project_name + ".vcxproj"), err);
   if (target_file.is_null())
     return false;
 
   base::FilePath vcxproj_path = build_settings_->GetFullPath(target_file);
   std::string vcxproj_path_str = FilePathToUTF8(vcxproj_path);
@@ skipping 479 matching lines @@
   }
 }
 
 std::string VisualStudioWriter::GetNinjaTarget(const Target* target) {
   std::ostringstream ninja_target_out;
   DCHECK(!target->dependency_output_file().value().empty());
   ninja_path_output_.WriteFile(ninja_target_out,
                                target->dependency_output_file());
   return ninja_target_out.str();
 }