Save the arguments order (#124)

Also upgrade `args_parser` to print generated command line from daemon

.clang-format

@@ -1,6 +1,9 @@
 BasedOnStyle: google
+Language: Cpp
+
 AllowShortBlocksOnASingleLine: false
 AllowShortFunctionsOnASingleLine: Inline
 AllowShortIfStatementsOnASingleLine: false
 AllowShortLoopsOnASingleLine: false
+ColumnLimit: 120
 DerivePointerAlignment: false

.gitignore

@@ -1,6 +1,7 @@
 *.autosave
 *.pyc
-/out
+/out/
 /*.creator.user*
 /.project
 *.swp
+/.ccls-cache/

src/base/base.proto

@@ -13,43 +13,49 @@ message Compiler {
   repeated Plugin plugins = 3;
 }
 
+message Argument {
+  optional uint32 index  = 1;
+  repeated string values = 2;
+}
+
+// TODO: migrate all arguments to |Argument| message type.
 message Flags {
-  required Compiler compiler   = 1;
+  required Compiler compiler         = 1;
   // original compiler executable.
 
-  optional string output       = 2;
+  optional string output             = 2;
   // "-o" flag and it's param.
 
-  optional string input        = 3;
+  optional string input              = 3;
   // relative path in the last argument.
 
-  repeated string other        = 4;
+  repeated Argument other            = 4;
   // all unsorted flags.
 
-  optional string deps_file    = 5;
+  optional string deps_file          = 5;
 
-  optional string language     = 6;
+  optional string language           = 6;
   // absorbs "-x" flag and stores it's param.
 
-  repeated string non_cached   = 7;
+  repeated Argument non_cached       = 7;
   // flags, that shouldn't be cached.
 
-  repeated string cc_only      = 8;
+  repeated Argument cc_only          = 8;
   // flags for compilation only.
 
-  required string action       = 9;
+  required string action             = 9;
   // "-emit-obj", "-E", etc.
 
-  repeated string non_direct   = 10;
+  repeated Argument non_direct       = 10;
   // flags, that shouldn't be neither direct cached, nor cached.
 
   optional string sanitize_blacklist = 11;
   // sanitizer blacklist file
 
-  repeated string included_files = 12;
-  // PCH or PTH files, included implicitly through command line arguments.
+  repeated string included_files     = 12;
+  // PCH or PTH files, included explicitly through command line arguments.
 
-  optional bool rewrite_includes = 13;
+  optional bool rewrite_includes     = 13;
   // Indicates if preprocessing should/was made using 'frewrite-includes' flag.
 
   // Compilation = All

src/client/clang_command.cc

@@ -66,17 +66,23 @@ bool ClangCommand::FillFlags(base::proto::Flags* flags,
 
   flags->Clear();
 
-  llvm::opt::ArgStringList non_direct_list, non_cached_list, other_list;
+  using ArgStringList = llvm::opt::ArgStringList;
+  dist_clang::List<Pair<ui32, ArgStringList>> non_direct_list, non_cached_list,
+      other_list;
   llvm::opt::DerivedArgList tmp_list(arg_list_);
 
+  ui32 index = 0;
   for (const auto& arg : arg_list_) {
+    ++index;
+
     // TODO: try to sort out flags by some attribute, i.e. group actions,
     //       compilation-only flags, etc.
 
     if (arg->getOption().getKind() == llvm::opt::Option::InputClass) {
       flags->set_input(arg->getValue());
     } else if (arg->getOption().matches(OPT_add_plugin)) {
-      arg->render(arg_list_, other_list);
+      arg->render(arg_list_,
+                  other_list.emplace_back(index, ArgStringList()).second);
       flags->mutable_compiler()->add_plugins()->set_name(arg->getValue());
     } else if (arg->getOption().matches(OPT_emit_obj) ||
                arg->getOption().matches(OPT_E) ||
@@ -88,7 +94,9 @@ bool ClangCommand::FillFlags(base::proto::Flags* flags,
     } else if (arg->getOption().matches(OPT_load)) {
       // FIXME: maybe claim this type of args right after generation?
     } else if (arg->getOption().matches(OPT_mrelax_all)) {
-      flags->add_cc_only(arg->getSpelling());
+      auto* new_arg = flags->add_cc_only();
+      new_arg->set_index(index);
+      new_arg->add_values(arg->getSpelling());
     } else if (arg->getOption().matches(OPT_o)) {
       flags->set_output(arg->getValue());
     } else if (arg->getOption().matches(OPT_x)) {
@@ -105,12 +113,16 @@ bool ClangCommand::FillFlags(base::proto::Flags* flags,
              arg->getOption().matches(OPT_internal_externc_isystem) ||
              arg->getOption().matches(OPT_isysroot) ||
              arg->getOption().matches(OPT_I)) {
-      arg->render(arg_list_, non_cached_list);
+      arg->render(arg_list_,
+                  non_cached_list.emplace_back(index, ArgStringList()).second);
     } else if (arg->getOption().matches(OPT_D)) {
       if (rewrite_includes) {
-        arg->render(arg_list_, other_list);
+        arg->render(arg_list_,
+                    other_list.emplace_back(index, ArgStringList()).second);
       } else {
-        arg->render(arg_list_, non_cached_list);
+        arg->render(
+            arg_list_,
+            non_cached_list.emplace_back(index, ArgStringList()).second);
       }
     } else if (arg->getOption().matches(OPT_include_pch) ||
                arg->getOption().matches(OPT_include_pth)) {
@@ -119,7 +131,8 @@ bool ClangCommand::FillFlags(base::proto::Flags* flags,
       // FIXME: don't render arguments here - render them somewhere in
       //        |CompilationDaemon|, but for now we don't pass the argument
       //        type.
-      arg->render(arg_list_, non_cached_list);
+      arg->render(arg_list_,
+                  non_cached_list.emplace_back(index, ArgStringList()).second);
     } else if (arg->getOption().matches(OPT_coverage_data_file) ||
                arg->getOption().matches(OPT_coverage_notes_file) ||
                arg->getOption().matches(OPT_fdebug_compilation_dir) ||
@@ -128,7 +141,8 @@ bool ClangCommand::FillFlags(base::proto::Flags* flags,
                arg->getOption().matches(OPT_MF) ||
                arg->getOption().matches(OPT_MMD) ||
                arg->getOption().matches(OPT_MT)) {
-      arg->render(arg_list_, non_direct_list);
+      arg->render(arg_list_,
+                  non_direct_list.emplace_back(index, ArgStringList()).second);
     } else if (arg->getOption().matches(OPT_internal_isystem) ||
                arg->getOption().matches(OPT_resource_dir)) {
       // Relative -internal-isystem and -resource_dir are based on current
@@ -158,13 +172,17 @@ bool ClangCommand::FillFlags(base::proto::Flags* flags,
         replaced_command.replace(
             pos, self_path.size(),
             clang_path.substr(0, clang_path.find_last_of('/')));
-        non_direct_list.push_back(arg->getSpelling().data());
-        non_direct_list.push_back(tmp_list.MakeArgString(replaced_command));
-        LOG(VERBOSE) << "Replaced command: " << non_direct_list.back();
+        auto& new_arg_list =
+            non_direct_list.emplace_back(index, ArgStringList()).second;
+        new_arg_list.push_back(arg->getSpelling().data());
+        new_arg_list.push_back(tmp_list.MakeArgString(replaced_command));
+        LOG(VERBOSE) << "Replaced command: " << new_arg_list.back();
       } else {
-        non_cached_list.push_back(arg->getSpelling().data());
-        non_cached_list.push_back(tmp_list.MakeArgString(replaced_command));
-        LOG(VERBOSE) << "Replaced command: " << non_cached_list.back();
+        auto& new_arg_list =
+            non_cached_list.emplace_back(index, ArgStringList()).second;
+        new_arg_list.push_back(arg->getSpelling().data());
+        new_arg_list.push_back(tmp_list.MakeArgString(replaced_command));
+        LOG(VERBOSE) << "Replaced command: " << new_arg_list.back();
       }
     }
 
@@ -178,30 +196,47 @@ bool ClangCommand::FillFlags(base::proto::Flags* flags,
                         return arg->getOption().matches(ignored_flag);
                       });
       if (!ignored) {
-        arg->render(arg_list_, other_list);
+        arg->render(arg_list_,
+                    other_list.emplace_back(index, ArgStringList()).second);
       }
     }
   }
 
+  // FIXME: this code smells - refactor it.
   // Leave |option_name| out of 'if' scope to make sure pointer copied to
   // |other_list| is alive.
   String option_name;
   if (rewrite_includes) {
     const auto& option = opts_->getOption(OPT_frewrite_includes);
     option_name = tmp_list.MakeArgString(option.getRenderName());
     option_name.insert(0, 1, '-');
-    other_list.push_back(option_name.c_str());
+    auto& new_arg_list = other_list.emplace_back(index, ArgStringList()).second;
+    new_arg_list.push_back(option_name.c_str());
   }
   flags->set_rewrite_includes(rewrite_includes);
 
-  for (const auto& value : non_direct_list) {
-    flags->add_non_direct(value);
+  for (const auto& arg_list : non_direct_list) {
+    auto* new_arg = flags->add_non_direct();
+    new_arg->set_index(arg_list.first);
+    for (const auto& arg : arg_list.second) {
+      new_arg->add_values(arg);
+    }
   }
-  for (const auto& value : non_cached_list) {
-    flags->add_non_cached(value);
+
+  for (const auto& arg_list : non_cached_list) {
+    auto* new_arg = flags->add_non_cached();
+    new_arg->set_index(arg_list.first);
+    for (const auto& arg : arg_list.second) {
+      new_arg->add_values(arg);
+    }
   }
-  for (const auto& value : other_list) {
-    flags->add_other(value);
+
+  for (const auto& arg_list : other_list) {
+    auto* new_arg = flags->add_other();
+    new_arg->set_index(arg_list.first);
+    for (const auto& arg : arg_list.second) {
+      new_arg->add_values(arg);
+    }
   }
 
   return true;

src/client/clang_test.cc

@@ -192,32 +192,52 @@ TEST_F(ClientTest, CannotReadMessage) {
       EXPECT_EQ("c++", cc_flags.language());
       EXPECT_EQ("-emit-obj", cc_flags.action());
 
+      auto find_value = [](const base::proto::Argument& arg,
+                           const char* value) {
+        return std::find(arg.values().begin(), arg.values().end(), value) !=
+               arg.values().end();
+      };
+      using namespace std::placeholders;
+
       {
         const auto& other = cc_flags.other();
         auto begin = other.begin();
         auto end = other.end();
-        EXPECT_NE(end, std::find(begin, end, "-cc1"));
-        EXPECT_NE(end, std::find(begin, end, "-triple"));
-        EXPECT_NE(end, std::find(begin, end, "-target-cpu"));
+        EXPECT_NE(end,
+                  std::find_if(begin, end, std::bind(find_value, _1, "-cc1")));
+        EXPECT_NE(end, std::find_if(begin, end,
+                                    std::bind(find_value, _1, "-triple")));
+        EXPECT_NE(end, std::find_if(begin, end,
+                                    std::bind(find_value, _1, "-target-cpu")));
       }
 
       {
         const auto& non_cached = cc_flags.non_cached();
         auto begin = non_cached.begin();
         auto end = non_cached.end();
 #if defined(OS_LINUX)
-        EXPECT_NE(end, std::find(begin, end, "-internal-externc-isystem"));
-        EXPECT_NE(end, std::find(begin, end, "-internal-isystem"));
+        EXPECT_NE(end, std::find_if(begin, end,
+                                    std::bind(find_value, _1,
+                                              "-internal-externc-isystem")));
+        EXPECT_NE(end,
+                  std::find_if(begin, end,
+                               std::bind(find_value, _1, "-internal-isystem")));
 #endif  // defined(OS_LINUX)
-        EXPECT_NE(end, std::find(begin, end, "-resource-dir"));
+        EXPECT_NE(end,
+                  std::find_if(begin, end,
+                               std::bind(find_value, _1, "-resource-dir")));
       }
 
       {
         const auto& non_direct = cc_flags.non_direct();
         auto begin = non_direct.begin();
         auto end = non_direct.end();
-        EXPECT_NE(end, std::find(begin, end, "-main-file-name"));
-        EXPECT_NE(end, std::find(begin, end, "-coverage-notes-file"));
+        EXPECT_NE(end,
+                  std::find_if(begin, end,
+                               std::bind(find_value, _1, "-main-file-name")));
+        EXPECT_NE(end, std::find_if(
+                           begin, end,
+                           std::bind(find_value, _1, "-coverage-notes-file")));
       }
     });
   };

src/client/command_test.cc

@@ -155,7 +155,7 @@ TEST(CommandTest, ParseCC1Args) {
   // Unknown arguments should go to |other| flags.
   bool found = false;
   for (const auto& flag : flags.other()) {
-    if (flag == "-really_unknown_arg") {
+    if (flag.values_size() && flag.values(0) == "-really_unknown_arg") {
       found = true;
       break;
     }
@@ -188,24 +188,30 @@ TEST(CommandTest, FillFlags) {
   EXPECT_EQ(output, flags.output());
   EXPECT_EQ("-emit-obj", flags.action());
   EXPECT_EQ("c++", flags.language());
-  EXPECT_EQ("-cc1", *flags.other().begin());
+  EXPECT_EQ("-cc1", flags.other().begin()->values(0));
   EXPECT_EQ(1, flags.compiler().plugins_size());
   EXPECT_EQ(plugin_name, flags.compiler().plugins(0).name());
 
-  // Check that macroses go to non-cached list.
+  auto find_value = [](const base::proto::Argument& arg, const char* value) {
+    return std::find(arg.values().begin(), arg.values().end(), value) !=
+           arg.values().end();
+  };
+  using namespace std::placeholders;
+
+  // Check that macros go to non-cached list.
   EXPECT_NE(flags.non_cached().end(),
-            std::find(flags.non_cached().begin(), flags.non_cached().end(),
-                      "MACRO"));
+            std::find_if(flags.non_cached().begin(), flags.non_cached().end(),
+                         std::bind(find_value, _1, "MACRO")));
 
   // Make sure -frewrite-includes removed if not used explicitly.
   EXPECT_EQ(flags.other().end(),
-            std::find(flags.other().begin(), flags.other().end(),
-                      "-frewrite-includes"));
+            std::find_if(flags.other().begin(), flags.other().end(),
+                         std::bind(find_value, _1, "-frewrite-includes")));
 
-  // Also ensure macroses don't go to other list.
+  // Also ensure macros don't go to other list.
   EXPECT_EQ(flags.other().end(),
-            std::find(flags.other().begin(), flags.other().end(),
-                      "MACRO"));
+            std::find_if(flags.other().begin(), flags.other().end(),
+                         std::bind(find_value, _1, "MACRO")));
 
   EXPECT_FALSE(flags.rewrite_includes());
 
@@ -238,23 +244,29 @@ TEST(CommandTest, FillFlagsAppendsRewriteIncludes) {
   EXPECT_EQ(output, flags.output());
   EXPECT_EQ("-emit-obj", flags.action());
   EXPECT_EQ("c++", flags.language());
-  EXPECT_EQ("-cc1", *flags.other().begin());
+  EXPECT_EQ("-cc1", flags.other().begin()->values(0));
+
+  auto find_value = [](const base::proto::Argument& arg, const char* value) {
+    return std::find(arg.values().begin(), arg.values().end(), value) !=
+           arg.values().end();
+  };
+  using namespace std::placeholders;
 
   // Check that macroses do not go to non-cached list.
   EXPECT_EQ(flags.non_cached().end(),
-            std::find(flags.non_cached().begin(), flags.non_cached().end(),
-                      "MACRO"));
+            std::find_if(flags.non_cached().begin(), flags.non_cached().end(),
+                         std::bind(find_value, _1, "MACRO")));
 
   // Make sure -frewrite-includes is added to other flags so it get passed to
   // preprocessor.
   EXPECT_NE(flags.other().end(),
-            std::find(flags.other().begin(), flags.other().end(),
-                      "-frewrite-includes"));
+            std::find_if(flags.other().begin(), flags.other().end(),
+                         std::bind(find_value, _1, "-frewrite-includes")));
 
   // Also ensure macroses go to other list.
   EXPECT_NE(flags.other().end(),
-            std::find(flags.other().begin(), flags.other().end(),
-                      "MACRO"));
+            std::find_if(flags.other().begin(), flags.other().end(),
+                         std::bind(find_value, _1, "MACRO")));
 
   // Check that flag indicating rewrite-includes was set.
   EXPECT_TRUE(flags.rewrite_includes());

src/daemon/BUILD.gn

@@ -15,7 +15,10 @@ executable("clangd") {
 
 # Separate source set for unit-tests.
 source_set("daemon") {
-  visibility += [ "//src/test:unit_tests" ]
+  visibility += [
+    "//src/test:unit_tests",
+    "//tools/args_parser:*",
+  ]
 
   sources = [
     "absorber.cc",