From 17b5ebd383df4c0c731b5c687ccaa3608b2d188f Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Wed, 11 Feb 2015 20:47:16 +0100 Subject: cmMacroCommand: Join the args strings outside of the loops. This means that we compute the strings even if not used in the macro but this shouldn't be expensive and it simplifies the code. --- Source/cmMacroCommand.cxx | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/Source/cmMacroCommand.cxx b/Source/cmMacroCommand.cxx index 8f6364d..de52fc9 100644 --- a/Source/cmMacroCommand.cxx +++ b/Source/cmMacroCommand.cxx @@ -114,6 +114,10 @@ bool cmMacroHelperCommand::InvokeInitialPass // declare varuiables for ARGV ARGN but do not compute until needed std::string argvDef; std::string argnDef; + std::vector::const_iterator eit + = expandedArgs.begin() + (this->Args.size() - 1); + std::string expandedArgn = cmJoin(cmRange(eit, expandedArgs.end()), ";"); + std::string expandedArgv = cmJoin(expandedArgs, ";"); bool argnDefInitialized = false; bool argvDefInitialized = false; if(!this->Functions.empty()) @@ -170,9 +174,7 @@ bool cmMacroHelperCommand::InvokeInitialPass { argnDef += ";"; } - std::vector::const_iterator eit - = expandedArgs.begin() + (this->Args.size() - 1); - argnDef += cmJoin(cmRange(eit, expandedArgs.end()), ";"); + argnDef += expandedArgn; } argnDefInitialized = true; } @@ -192,7 +194,7 @@ bool cmMacroHelperCommand::InvokeInitialPass { argvDef += ";"; } - argvDef += cmJoin(expandedArgs, ";"); + argvDef += expandedArgv; argvDefInitialized = true; } cmSystemTools::ReplaceString(tmps, "${ARGV}", argvDef.c_str()); -- cgit v0.12 From 081a13f7c011d0ebd28caebfedee1e3e3f7602c9 Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Wed, 11 Feb 2015 19:10:19 +0100 Subject: cmMacroCommand: Declare arg variables where used and initialized. Make the initialization by population with the expanded* content unconditional. --- Source/cmMacroCommand.cxx | 30 +++++++++--------------------- 1 file changed, 9 insertions(+), 21 deletions(-) diff --git a/Source/cmMacroCommand.cxx b/Source/cmMacroCommand.cxx index de52fc9..f7daa76 100644 --- a/Source/cmMacroCommand.cxx +++ b/Source/cmMacroCommand.cxx @@ -111,15 +111,10 @@ bool cmMacroHelperCommand::InvokeInitialPass argcDefStream << expandedArgs.size(); std::string argcDef = argcDefStream.str(); - // declare varuiables for ARGV ARGN but do not compute until needed - std::string argvDef; - std::string argnDef; std::vector::const_iterator eit = expandedArgs.begin() + (this->Args.size() - 1); std::string expandedArgn = cmJoin(cmRange(eit, expandedArgs.end()), ";"); std::string expandedArgv = cmJoin(expandedArgs, ";"); - bool argnDefInitialized = false; - bool argvDefInitialized = false; if(!this->Functions.empty()) { this->FilePath = this->Functions[0].FilePath; @@ -166,17 +161,14 @@ bool cmMacroHelperCommand::InvokeInitialPass // repleace ARGN if (tmps.find("${ARGN}") != std::string::npos) { - if (!argnDefInitialized) + std::string argnDef; + if (expandedArgs.size() > this->Args.size() - 1) { - if (expandedArgs.size() > this->Args.size() - 1) + if (!argnDef.empty() && !expandedArgs.empty()) { - if (!argnDef.empty() && !expandedArgs.empty()) - { - argnDef += ";"; - } - argnDef += expandedArgn; + argnDef += ";"; } - argnDefInitialized = true; + argnDef += expandedArgn; } cmSystemTools::ReplaceString(tmps, "${ARGN}", argnDef.c_str()); } @@ -187,16 +179,12 @@ bool cmMacroHelperCommand::InvokeInitialPass { char argvName[60]; - // repleace ARGV, compute it only once - if (!argvDefInitialized) + std::string argvDef; + if (!argvDef.empty() && !expandedArgs.empty()) { - if (!argvDef.empty() && !expandedArgs.empty()) - { - argvDef += ";"; - } - argvDef += expandedArgv; - argvDefInitialized = true; + argvDef += ";"; } + argvDef += expandedArgv; cmSystemTools::ReplaceString(tmps, "${ARGV}", argvDef.c_str()); // also replace the ARGV1 ARGV2 ... etc -- cgit v0.12 From 3250a7e535684bec684c0109b31675ba72dc3aa9 Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Wed, 11 Feb 2015 19:16:15 +0100 Subject: cmMacroCommand: Remove conditional append of semicolon. The conditions are never true. --- Source/cmMacroCommand.cxx | 8 -------- 1 file changed, 8 deletions(-) diff --git a/Source/cmMacroCommand.cxx b/Source/cmMacroCommand.cxx index f7daa76..b27b994 100644 --- a/Source/cmMacroCommand.cxx +++ b/Source/cmMacroCommand.cxx @@ -164,10 +164,6 @@ bool cmMacroHelperCommand::InvokeInitialPass std::string argnDef; if (expandedArgs.size() > this->Args.size() - 1) { - if (!argnDef.empty() && !expandedArgs.empty()) - { - argnDef += ";"; - } argnDef += expandedArgn; } cmSystemTools::ReplaceString(tmps, "${ARGN}", argnDef.c_str()); @@ -180,10 +176,6 @@ bool cmMacroHelperCommand::InvokeInitialPass char argvName[60]; std::string argvDef; - if (!argvDef.empty() && !expandedArgs.empty()) - { - argvDef += ";"; - } argvDef += expandedArgv; cmSystemTools::ReplaceString(tmps, "${ARGV}", argvDef.c_str()); -- cgit v0.12 From f2c49f59d8dbca936a40b62a9079fa71ca551365 Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Wed, 11 Feb 2015 19:17:29 +0100 Subject: cmMacroCommand: Remove condition around ARGN computation. An empty string is appended if the condition is false, which is ok for this commit. --- Source/cmMacroCommand.cxx | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/Source/cmMacroCommand.cxx b/Source/cmMacroCommand.cxx index b27b994..9fc479e 100644 --- a/Source/cmMacroCommand.cxx +++ b/Source/cmMacroCommand.cxx @@ -162,10 +162,7 @@ bool cmMacroHelperCommand::InvokeInitialPass if (tmps.find("${ARGN}") != std::string::npos) { std::string argnDef; - if (expandedArgs.size() > this->Args.size() - 1) - { - argnDef += expandedArgn; - } + argnDef += expandedArgn; cmSystemTools::ReplaceString(tmps, "${ARGN}", argnDef.c_str()); } -- cgit v0.12 From 8e0827b646b14028446503ac392a9ab7bb5a53a3 Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Wed, 11 Feb 2015 19:26:33 +0100 Subject: cmMacroCommand: Remove intermediate arg variables. --- Source/cmMacroCommand.cxx | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/Source/cmMacroCommand.cxx b/Source/cmMacroCommand.cxx index 9fc479e..8bbb7c6 100644 --- a/Source/cmMacroCommand.cxx +++ b/Source/cmMacroCommand.cxx @@ -161,9 +161,7 @@ bool cmMacroHelperCommand::InvokeInitialPass // repleace ARGN if (tmps.find("${ARGN}") != std::string::npos) { - std::string argnDef; - argnDef += expandedArgn; - cmSystemTools::ReplaceString(tmps, "${ARGN}", argnDef.c_str()); + cmSystemTools::ReplaceString(tmps, "${ARGN}", expandedArgn.c_str()); } // if the current argument of the current function has ${ARGV in it @@ -171,10 +169,7 @@ bool cmMacroHelperCommand::InvokeInitialPass if (tmps.find("${ARGV") != std::string::npos) { char argvName[60]; - - std::string argvDef; - argvDef += expandedArgv; - cmSystemTools::ReplaceString(tmps, "${ARGV}", argvDef.c_str()); + cmSystemTools::ReplaceString(tmps, "${ARGV}", expandedArgv.c_str()); // also replace the ARGV1 ARGV2 ... etc for (unsigned int t = 0; t < expandedArgs.size(); ++t) -- cgit v0.12 From f79c0f7697ee0bd25ec74f7dbb2ac0cf6189a9df Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Wed, 11 Feb 2015 19:51:15 +0100 Subject: cmMacroCommand: Compute variables outside of two loops. Avoid computing them from scratch for each argument of each function. --- Source/cmMacroCommand.cxx | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/Source/cmMacroCommand.cxx b/Source/cmMacroCommand.cxx index 8bbb7c6..8424c75 100644 --- a/Source/cmMacroCommand.cxx +++ b/Source/cmMacroCommand.cxx @@ -115,6 +115,15 @@ bool cmMacroHelperCommand::InvokeInitialPass = expandedArgs.begin() + (this->Args.size() - 1); std::string expandedArgn = cmJoin(cmRange(eit, expandedArgs.end()), ";"); std::string expandedArgv = cmJoin(expandedArgs, ";"); + std::vector variables; + variables.reserve(this->Args.size() - 1); + for (unsigned int j = 1; j < this->Args.size(); ++j) + { + std::string variable = "${"; + variable += this->Args[j]; + variable += "}"; + variables.push_back(variable); + } if(!this->Functions.empty()) { this->FilePath = this->Functions[0].FilePath; @@ -147,13 +156,10 @@ bool cmMacroHelperCommand::InvokeInitialPass { tmps = k->Value; // replace formal arguments - for (unsigned int j = 1; j < this->Args.size(); ++j) + for (unsigned int j = 0; j < variables.size(); ++j) { - variable = "${"; - variable += this->Args[j]; - variable += "}"; - cmSystemTools::ReplaceString(tmps, variable.c_str(), - expandedArgs[j-1].c_str()); + cmSystemTools::ReplaceString(tmps, variables[j].c_str(), + expandedArgs[j].c_str()); } // replace argc cmSystemTools::ReplaceString(tmps, "${ARGC}",argcDef.c_str()); -- cgit v0.12 From a551851ab3f23f58074638ffcf38a08d98a35fa8 Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Wed, 11 Feb 2015 20:14:30 +0100 Subject: cmMacroCommand: Inline variable computation. --- Source/cmMacroCommand.cxx | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/Source/cmMacroCommand.cxx b/Source/cmMacroCommand.cxx index 8424c75..6f45484 100644 --- a/Source/cmMacroCommand.cxx +++ b/Source/cmMacroCommand.cxx @@ -86,7 +86,6 @@ bool cmMacroHelperCommand::InvokeInitialPass std::string tmps; cmListFileArgument arg; - std::string variable; // make sure the number of arguments passed is at least the number // required by the signature @@ -119,10 +118,7 @@ bool cmMacroHelperCommand::InvokeInitialPass variables.reserve(this->Args.size() - 1); for (unsigned int j = 1; j < this->Args.size(); ++j) { - std::string variable = "${"; - variable += this->Args[j]; - variable += "}"; - variables.push_back(variable); + variables.push_back("${" + this->Args[j] + "}"); } if(!this->Functions.empty()) { -- cgit v0.12 From 2c4a7298fcb3ac6a6b6c0a9545b343c814f3d6ec Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Wed, 11 Feb 2015 20:23:36 +0100 Subject: cmMacroCommand: Declare arg in the scope that it is used. It can make sense to declare objects outside of loops if the size required by the object can grow (eg std::string when using getline), but that is not the case here. --- Source/cmMacroCommand.cxx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Source/cmMacroCommand.cxx b/Source/cmMacroCommand.cxx index 6f45484..bd53b1e 100644 --- a/Source/cmMacroCommand.cxx +++ b/Source/cmMacroCommand.cxx @@ -85,7 +85,6 @@ bool cmMacroHelperCommand::InvokeInitialPass this->Makefile->ExpandArguments(args, expandedArgs); std::string tmps; - cmListFileArgument arg; // make sure the number of arguments passed is at least the number // required by the signature @@ -144,6 +143,8 @@ bool cmMacroHelperCommand::InvokeInitialPass // Set the FilePath on the arguments to match the function since it is // not stored and the original values may be freed k->FilePath = this->FilePath.c_str(); + + cmListFileArgument arg; if(k->Delim == cmListFileArgument::Bracket) { arg.Value = k->Value; -- cgit v0.12 From 6774c92b5809b80c767ef8094b2f26d06556e0fd Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Wed, 11 Feb 2015 20:25:44 +0100 Subject: cmMacroCommand: Declare tmps in the scope that it's used. We don't particularly need to reuse the string memory here, and this pattern is not common in CMake. --- Source/cmMacroCommand.cxx | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/Source/cmMacroCommand.cxx b/Source/cmMacroCommand.cxx index bd53b1e..6635fee 100644 --- a/Source/cmMacroCommand.cxx +++ b/Source/cmMacroCommand.cxx @@ -84,8 +84,6 @@ bool cmMacroHelperCommand::InvokeInitialPass std::vector expandedArgs; this->Makefile->ExpandArguments(args, expandedArgs); - std::string tmps; - // make sure the number of arguments passed is at least the number // required by the signature if (expandedArgs.size() < this->Args.size() - 1) @@ -151,7 +149,7 @@ bool cmMacroHelperCommand::InvokeInitialPass } else { - tmps = k->Value; + std::string tmps = k->Value; // replace formal arguments for (unsigned int j = 0; j < variables.size(); ++j) { -- cgit v0.12 From 4aa7bd2ac11aa2edb0288e5ba360984b7799130d Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Wed, 11 Feb 2015 20:06:39 +0100 Subject: cmMacroCommand: Remove condition around ARGN replacement. There is none for ARGC replacement, so no reason to conditionalize the replacement. The computation is already done. --- Source/cmMacroCommand.cxx | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/Source/cmMacroCommand.cxx b/Source/cmMacroCommand.cxx index 6635fee..111ad96 100644 --- a/Source/cmMacroCommand.cxx +++ b/Source/cmMacroCommand.cxx @@ -159,11 +159,7 @@ bool cmMacroHelperCommand::InvokeInitialPass // replace argc cmSystemTools::ReplaceString(tmps, "${ARGC}",argcDef.c_str()); - // repleace ARGN - if (tmps.find("${ARGN}") != std::string::npos) - { - cmSystemTools::ReplaceString(tmps, "${ARGN}", expandedArgn.c_str()); - } + cmSystemTools::ReplaceString(tmps, "${ARGN}", expandedArgn.c_str()); // if the current argument of the current function has ${ARGV in it // then try replacing ARGV values -- cgit v0.12 From 9a1f8f35f48d7dcfefc70de9ae530c31b16cd9e0 Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Wed, 11 Feb 2015 20:08:05 +0100 Subject: cmMacroCommand: Move ARGV replacement out of condition. --- Source/cmMacroCommand.cxx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Source/cmMacroCommand.cxx b/Source/cmMacroCommand.cxx index 111ad96..44b1465 100644 --- a/Source/cmMacroCommand.cxx +++ b/Source/cmMacroCommand.cxx @@ -160,14 +160,13 @@ bool cmMacroHelperCommand::InvokeInitialPass cmSystemTools::ReplaceString(tmps, "${ARGC}",argcDef.c_str()); cmSystemTools::ReplaceString(tmps, "${ARGN}", expandedArgn.c_str()); + cmSystemTools::ReplaceString(tmps, "${ARGV}", expandedArgv.c_str()); // if the current argument of the current function has ${ARGV in it // then try replacing ARGV values if (tmps.find("${ARGV") != std::string::npos) { char argvName[60]; - cmSystemTools::ReplaceString(tmps, "${ARGV}", expandedArgv.c_str()); - // also replace the ARGV1 ARGV2 ... etc for (unsigned int t = 0; t < expandedArgs.size(); ++t) { -- cgit v0.12 From 83414d5a07753d004f5e663c385ef0c713895d46 Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Wed, 11 Feb 2015 20:16:40 +0100 Subject: cmMacroCommand: Move computation of ARGV%n names out of double loop. --- Source/cmMacroCommand.cxx | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/Source/cmMacroCommand.cxx b/Source/cmMacroCommand.cxx index 44b1465..49b9fd2 100644 --- a/Source/cmMacroCommand.cxx +++ b/Source/cmMacroCommand.cxx @@ -117,6 +117,14 @@ bool cmMacroHelperCommand::InvokeInitialPass { variables.push_back("${" + this->Args[j] + "}"); } + std::vector argVs; + argVs.reserve(expandedArgs.size()); + char argvName[60]; + for (unsigned int j = 0; j < expandedArgs.size(); ++j) + { + sprintf(argvName,"${ARGV%i}",j); + argVs.push_back(argvName); + } if(!this->Functions.empty()) { this->FilePath = this->Functions[0].FilePath; @@ -166,12 +174,9 @@ bool cmMacroHelperCommand::InvokeInitialPass // then try replacing ARGV values if (tmps.find("${ARGV") != std::string::npos) { - char argvName[60]; - // also replace the ARGV1 ARGV2 ... etc for (unsigned int t = 0; t < expandedArgs.size(); ++t) { - sprintf(argvName,"${ARGV%i}",t); - cmSystemTools::ReplaceString(tmps, argvName, + cmSystemTools::ReplaceString(tmps, argVs[t].c_str(), expandedArgs[t].c_str()); } } -- cgit v0.12 From b5f98e5012ef53ee6746c42131fc66892bfff717 Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Wed, 11 Feb 2015 22:16:03 +0100 Subject: cmMacroCommand: Manipulate target string directly. Avoid copying a string from the source, manipulating it, and then copying it back. Manipulate it in place instead. --- Source/cmMacroCommand.cxx | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/Source/cmMacroCommand.cxx b/Source/cmMacroCommand.cxx index 49b9fd2..29e8cb1 100644 --- a/Source/cmMacroCommand.cxx +++ b/Source/cmMacroCommand.cxx @@ -151,37 +151,33 @@ bool cmMacroHelperCommand::InvokeInitialPass k->FilePath = this->FilePath.c_str(); cmListFileArgument arg; - if(k->Delim == cmListFileArgument::Bracket) + arg.Value = k->Value; + if(k->Delim != cmListFileArgument::Bracket) { - arg.Value = k->Value; - } - else - { - std::string tmps = k->Value; // replace formal arguments for (unsigned int j = 0; j < variables.size(); ++j) { - cmSystemTools::ReplaceString(tmps, variables[j].c_str(), + cmSystemTools::ReplaceString(arg.Value, variables[j].c_str(), expandedArgs[j].c_str()); } // replace argc - cmSystemTools::ReplaceString(tmps, "${ARGC}",argcDef.c_str()); + cmSystemTools::ReplaceString(arg.Value, "${ARGC}",argcDef.c_str()); - cmSystemTools::ReplaceString(tmps, "${ARGN}", expandedArgn.c_str()); - cmSystemTools::ReplaceString(tmps, "${ARGV}", expandedArgv.c_str()); + cmSystemTools::ReplaceString(arg.Value, "${ARGN}", + expandedArgn.c_str()); + cmSystemTools::ReplaceString(arg.Value, "${ARGV}", + expandedArgv.c_str()); // if the current argument of the current function has ${ARGV in it // then try replacing ARGV values - if (tmps.find("${ARGV") != std::string::npos) + if (arg.Value.find("${ARGV") != std::string::npos) { for (unsigned int t = 0; t < expandedArgs.size(); ++t) { - cmSystemTools::ReplaceString(tmps, argVs[t].c_str(), + cmSystemTools::ReplaceString(arg.Value, argVs[t].c_str(), expandedArgs[t].c_str()); } } - - arg.Value = tmps; } arg.Delim = k->Delim; arg.FilePath = k->FilePath; -- cgit v0.12