Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Save the arguments order #124

Merged
merged 2 commits into from
Feb 1, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .clang-format
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
BasedOnStyle: google
Language: Cpp

AllowShortBlocksOnASingleLine: false
AllowShortFunctionsOnASingleLine: Inline
AllowShortIfStatementsOnASingleLine: false
AllowShortLoopsOnASingleLine: false
ColumnLimit: 120
DerivePointerAlignment: false
3 changes: 2 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
*.autosave
*.pyc
/out
/out/
/*.creator.user*
/.project
*.swp
/.ccls-cache/
File renamed without changes.
32 changes: 19 additions & 13 deletions src/base/base.proto
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
79 changes: 57 additions & 22 deletions src/client/clang_command.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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) ||
Expand All @@ -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)) {
Expand All @@ -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)) {
Expand All @@ -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) ||
Expand All @@ -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
Expand Down Expand Up @@ -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();
}
}

Expand All @@ -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;
Expand Down
36 changes: 28 additions & 8 deletions src/client/clang_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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")));
}
});
};
Expand Down
46 changes: 29 additions & 17 deletions src/client/command_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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());

Expand Down Expand Up @@ -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());
Expand Down
5 changes: 4 additions & 1 deletion src/daemon/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Loading