CommandEvaluator: reduce needless allocations and clarify interface
When command vectors are populated by the caller as vector<Command*> and
passed as an input/output parameter via pointer. This interface is
problematic as the contract is not well defined. Clarify that by always
producing the vector within CommandEvaluator::Eval() and return it by
value. Further, the commands are usually only read and never ownership
is transferred. Hence let the vector manage the allocations and don't
bother about that any longer by passing vector<Command> instead.
Signed-off-by: Matthias Maennich <maennich@google.com>
diff --git a/src/command.cc b/src/command.cc
index 6b177ef..9d594f8 100644
--- a/src/command.cc
+++ b/src/command.cc
@@ -185,7 +185,8 @@
INSERT_AUTO_VAR(AutoNotImplementedVar, "|");
}
-void CommandEvaluator::Eval(DepNode* n, vector<Command*>* commands) {
+std::vector<Command> CommandEvaluator::Eval(DepNode* n) {
+ std::vector<Command> result;
ev_->set_loc(n->loc);
ev_->set_current_scope(n->rule_vars);
current_dep_node_ = n;
@@ -211,11 +212,10 @@
ParseCommandPrefixes(&cmd, &echo, &ignore_error);
if (!cmd.empty()) {
- Command* command = new Command(n->output);
- command->cmd = cmd.as_string();
- command->echo = echo;
- command->ignore_error = ignore_error;
- commands->push_back(command);
+ Command& command = result.emplace_back(n->output);
+ command.cmd = cmd.as_string();
+ command.echo = echo;
+ command.ignore_error = ignore_error;
}
if (index == string::npos)
break;
@@ -224,20 +224,20 @@
}
if (!ev_->delayed_output_commands().empty()) {
- vector<Command*> output_commands;
+ std::vector<Command> output_commands;
for (const string& cmd : ev_->delayed_output_commands()) {
- Command* c = new Command(n->output);
- c->cmd = cmd;
- c->echo = false;
- c->ignore_error = false;
- output_commands.push_back(c);
+ Command& c = output_commands.emplace_back(n->output);
+ c.cmd = cmd;
+ c.echo = false;
+ c.ignore_error = false;
}
// Prepend |output_commands|.
- commands->swap(output_commands);
- copy(output_commands.begin(), output_commands.end(),
- back_inserter(*commands));
+ result.swap(output_commands);
+ copy(output_commands.begin(), output_commands.end(), back_inserter(result));
ev_->clear_delayed_output_commands();
}
ev_->set_current_scope(NULL);
+
+ return result;
}
diff --git a/src/command.h b/src/command.h
index 081016d..e55ca93 100644
--- a/src/command.h
+++ b/src/command.h
@@ -35,7 +35,7 @@
class CommandEvaluator {
public:
explicit CommandEvaluator(Evaluator* ev);
- void Eval(DepNode* n, vector<Command*>* commands);
+ std::vector<Command> Eval(DepNode* n);
const DepNode* current_dep_node() const { return current_dep_node_; }
Evaluator* evaluator() const { return ev_; }
diff --git a/src/exec.cc b/src/exec.cc
index eab7222..6560301 100644
--- a/src/exec.cc
+++ b/src/exec.cc
@@ -98,31 +98,29 @@
return output_ts;
}
- vector<Command*> commands;
- ce_.Eval(n, &commands);
- for (Command* command : commands) {
+ auto commands = ce_.Eval(n);
+ for (const Command& command : commands) {
num_commands_ += 1;
- if (command->echo) {
- printf("%s\n", command->cmd.c_str());
+ if (command.echo) {
+ printf("%s\n", command.cmd.c_str());
fflush(stdout);
}
if (!g_flags.is_dry_run) {
string out;
- int result = RunCommand(shell_, shellflag_, command->cmd.c_str(),
+ int result = RunCommand(shell_, shellflag_, command.cmd.c_str(),
RedirectStderr::STDOUT, &out);
printf("%s", out.c_str());
if (result != 0) {
- if (command->ignore_error) {
- fprintf(stderr, "[%s] Error %d (ignored)\n",
- command->output.c_str(), WEXITSTATUS(result));
+ if (command.ignore_error) {
+ fprintf(stderr, "[%s] Error %d (ignored)\n", command.output.c_str(),
+ WEXITSTATUS(result));
} else {
- fprintf(stderr, "*** [%s] Error %d\n", command->output.c_str(),
+ fprintf(stderr, "*** [%s] Error %d\n", command.output.c_str(),
WEXITSTATUS(result));
exit(1);
}
}
}
- delete command;
}
done_[n->output] = output_ts;
diff --git a/src/ninja.cc b/src/ninja.cc
index bccba5f..2cdfb49 100644
--- a/src/ninja.cc
+++ b/src/ninja.cc
@@ -171,7 +171,7 @@
struct NinjaNode {
const DepNode* node;
- vector<Command*> commands;
+ vector<Command> commands;
int rule_id;
};
@@ -245,7 +245,7 @@
NinjaNode& nn = nodes_.emplace_back();
nn.node = node;
- ce_.Eval(node, &nn.commands);
+ nn.commands = ce_.Eval(node);
nn.rule_id = nn.commands.empty() ? -1 : rule_id_++;
for (auto const& d : node->deps) {
@@ -399,36 +399,35 @@
}
bool GenShellScript(const char* name,
- const vector<Command*>& commands,
+ const vector<Command>& commands,
string* cmd_buf,
string* description) {
bool got_descritpion = false;
bool use_gomacc = false;
auto command_count = commands.size();
- for (const Command* c : commands) {
+ for (const Command& c : commands) {
size_t cmd_begin = cmd_buf->size();
if (!cmd_buf->empty()) {
*cmd_buf += " && ";
}
- const char* in = c->cmd.c_str();
+ const char* in = c.cmd.c_str();
while (isspace(*in))
in++;
- bool needs_subshell = (command_count > 1 || c->ignore_error);
+ bool needs_subshell = (command_count > 1 || c.ignore_error);
if (needs_subshell)
*cmd_buf += '(';
size_t cmd_start = cmd_buf->size();
StringPiece translated = TranslateCommand(in, cmd_buf);
- if (g_flags.detect_android_echo && !got_descritpion && !c->echo &&
+ if (g_flags.detect_android_echo && !got_descritpion && !c.echo &&
GetDescriptionFromCommand(translated, description)) {
got_descritpion = true;
translated.clear();
- } else if (IsOutputMkdir(name, translated) && !c->echo &&
- cmd_begin == 0) {
+ } else if (IsOutputMkdir(name, translated) && !c.echo && cmd_begin == 0) {
translated.clear();
}
if (translated.empty()) {
@@ -445,7 +444,7 @@
use_gomacc = true;
}
- if (c->ignore_error) {
+ if (c.ignore_error) {
*cmd_buf += " ; true";
}
@@ -481,7 +480,7 @@
void EmitNode(const NinjaNode& nn, std::ostream& out) {
const DepNode* node = nn.node;
- const vector<Command*>& commands = nn.commands;
+ const vector<Command>& commands = nn.commands;
string rule_name = "phony";
bool use_local_pool = false;
