From b900c1ccaae7a500dda88240873122d0d899bf93 Mon Sep 17 00:00:00 2001 From: Nils Gladitz Date: Thu, 11 Sep 2014 19:50:51 +0200 Subject: If: Extract cmConditionEvaluator from if() implementation --- Source/cmBootstrapCommands2.cxx | 1 + Source/cmConditionEvaluator.cxx | 692 ++++++++++++++++++++++++++++++++++++++ Source/cmConditionEvaluator.h | 90 +++++ Source/cmIfCommand.cxx | 713 +--------------------------------------- Source/cmIfCommand.h | 12 - Source/cmWhileCommand.cxx | 15 +- 6 files changed, 803 insertions(+), 720 deletions(-) create mode 100644 Source/cmConditionEvaluator.cxx create mode 100644 Source/cmConditionEvaluator.h diff --git a/Source/cmBootstrapCommands2.cxx b/Source/cmBootstrapCommands2.cxx index be72526..91c189f 100644 --- a/Source/cmBootstrapCommands2.cxx +++ b/Source/cmBootstrapCommands2.cxx @@ -14,6 +14,7 @@ // This is sort of a boot strapping approach since you would // like to have CMake to build CMake. #include "cmCommands.h" +#include "cmConditionEvaluator.cxx" #include "cmGeneratorExpressionEvaluationFile.cxx" #include "cmGetCMakePropertyCommand.cxx" #include "cmGetDirectoryPropertyCommand.cxx" diff --git a/Source/cmConditionEvaluator.cxx b/Source/cmConditionEvaluator.cxx new file mode 100644 index 0000000..869d3c5 --- /dev/null +++ b/Source/cmConditionEvaluator.cxx @@ -0,0 +1,692 @@ +/*============================================================================ + CMake - Cross Platform Makefile Generator + Copyright 2014 Kitware, Inc., Insight Software Consortium + + Distributed under the OSI-approved BSD License (the "License"); + see accompanying file Copyright.txt for details. + + This software is distributed WITHOUT ANY WARRANTY; without even the + implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. + See the License for more information. +============================================================================*/ + +#include "cmConditionEvaluator.h" + +cmConditionEvaluator::cmConditionEvaluator(cmMakefile& makefile): + Makefile(makefile), + Policy12Status(makefile.GetPolicyStatus(cmPolicies::CMP0012)) +{ + +} + +//========================================================================= +// order of operations, +// 1. ( ) -- parenthetical groups +// 2. IS_DIRECTORY EXISTS COMMAND DEFINED etc predicates +// 3. MATCHES LESS GREATER EQUAL STRLESS STRGREATER STREQUAL etc binary ops +// 4. NOT +// 5. AND OR +// +// There is an issue on whether the arguments should be values of references, +// for example IF (FOO AND BAR) should that compare the strings FOO and BAR +// or should it really do IF (${FOO} AND ${BAR}) Currently IS_DIRECTORY +// EXISTS COMMAND and DEFINED all take values. EQUAL, LESS and GREATER can +// take numeric values or variable names. STRLESS and STRGREATER take +// variable names but if the variable name is not found it will use the name +// directly. AND OR take variables or the values 0 or 1. + +bool cmConditionEvaluator::IsTrue( + const std::vector &args, + std::string &errorString, + cmake::MessageType &status) +{ + errorString = ""; + + // handle empty invocation + if (args.size() < 1) + { + return false; + } + + // store the reduced args in this vector + cmArgumentList newArgs; + + // copy to the list structure + for(unsigned int i = 0; i < args.size(); ++i) + { + newArgs.push_back(args[i]); + } + + // now loop through the arguments and see if we can reduce any of them + // we do this multiple times. Once for each level of precedence + // parens + if (!this->HandleLevel0(newArgs, errorString, status)) + { + return false; + } + //predicates + if (!this->HandleLevel1(newArgs, errorString, status)) + { + return false; + } + // binary ops + if (!this->HandleLevel2(newArgs, errorString, status)) + { + return false; + } + + // NOT + if (!this->HandleLevel3(newArgs, errorString, status)) + { + return false; + } + // AND OR + if (!this->HandleLevel4(newArgs, errorString, status)) + { + return false; + } + + // now at the end there should only be one argument left + if (newArgs.size() != 1) + { + errorString = "Unknown arguments specified"; + status = cmake::FATAL_ERROR; + return false; + } + + return this->GetBooleanValueWithAutoDereference(*(newArgs.begin()), + errorString, status, true); +} + +//========================================================================= +const char* cmConditionEvaluator::GetVariableOrString( + const std::string& str) const +{ + const char* def = this->Makefile.GetDefinition(str); + + if(!def) + { + def = str.c_str(); + } + + return def; +} + +//========================================================================= +bool cmConditionEvaluator::GetBooleanValue( + std::string& arg) const +{ + // Check basic constants. + if (arg == "0") + { + return false; + } + if (arg == "1") + { + return true; + } + + // Check named constants. + if (cmSystemTools::IsOn(arg.c_str())) + { + return true; + } + if (cmSystemTools::IsOff(arg.c_str())) + { + return false; + } + + // Check for numbers. + if(!arg.empty()) + { + char* end; + double d = strtod(arg.c_str(), &end); + if(*end == '\0') + { + // The whole string is a number. Use C conversion to bool. + return d? true:false; + } + } + + // Check definition. + const char* def = this->Makefile.GetDefinition(arg); + return !cmSystemTools::IsOff(def); +} + +//========================================================================= +// Boolean value behavior from CMake 2.6.4 and below. +bool cmConditionEvaluator::GetBooleanValueOld( + std::string const& arg, bool one) const +{ + if(one) + { + // Old IsTrue behavior for single argument. + if(arg == "0") + { return false; } + else if(arg == "1") + { return true; } + else + { return !cmSystemTools::IsOff(this->Makefile.GetDefinition(arg)); } + } + else + { + // Old GetVariableOrNumber behavior. + const char* def = this->Makefile.GetDefinition(arg); + if(!def && atoi(arg.c_str())) + { + def = arg.c_str(); + } + return !cmSystemTools::IsOff(def); + } +} + +//========================================================================= +// returns the resulting boolean value +bool cmConditionEvaluator::GetBooleanValueWithAutoDereference( + std::string &newArg, + std::string &errorString, + cmake::MessageType &status, + bool oneArg) const +{ + // Use the policy if it is set. + if (this->Policy12Status == cmPolicies::NEW) + { + return GetBooleanValue(newArg); + } + else if (this->Policy12Status == cmPolicies::OLD) + { + return GetBooleanValueOld(newArg, oneArg); + } + + // Check policy only if old and new results differ. + bool newResult = this->GetBooleanValue(newArg); + bool oldResult = this->GetBooleanValueOld(newArg, oneArg); + if(newResult != oldResult) + { + switch(this->Policy12Status) + { + case cmPolicies::WARN: + { + cmPolicies* policies = this->Makefile.GetPolicies(); + errorString = "An argument named \"" + newArg + + "\" appears in a conditional statement. " + + policies->GetPolicyWarning(cmPolicies::CMP0012); + status = cmake::AUTHOR_WARNING; + } + case cmPolicies::OLD: + return oldResult; + case cmPolicies::REQUIRED_IF_USED: + case cmPolicies::REQUIRED_ALWAYS: + { + cmPolicies* policies = this->Makefile.GetPolicies(); + errorString = "An argument named \"" + newArg + + "\" appears in a conditional statement. " + + policies->GetRequiredPolicyError(cmPolicies::CMP0012); + status = cmake::FATAL_ERROR; + } + case cmPolicies::NEW: + break; + } + } + return newResult; +} + +//========================================================================= +void cmConditionEvaluator::IncrementArguments(cmArgumentList &newArgs, + cmArgumentList::iterator &argP1, + cmArgumentList::iterator &argP2) const +{ + if (argP1 != newArgs.end()) + { + argP1++; + argP2 = argP1; + if (argP1 != newArgs.end()) + { + argP2++; + } + } +} + +//========================================================================= +// helper function to reduce code duplication +void cmConditionEvaluator::HandlePredicate(bool value, int &reducible, + cmArgumentList::iterator &arg, + cmArgumentList &newArgs, + cmArgumentList::iterator &argP1, + cmArgumentList::iterator &argP2) const +{ + if(value) + { + *arg = "1"; + } + else + { + *arg = "0"; + } + newArgs.erase(argP1); + argP1 = arg; + this->IncrementArguments(newArgs,argP1,argP2); + reducible = 1; +} + +//========================================================================= +// helper function to reduce code duplication +void cmConditionEvaluator::HandleBinaryOp(bool value, int &reducible, + cmArgumentList::iterator &arg, + cmArgumentList &newArgs, + cmArgumentList::iterator &argP1, + cmArgumentList::iterator &argP2) +{ + if(value) + { + *arg = "1"; + } + else + { + *arg = "0"; + } + newArgs.erase(argP2); + newArgs.erase(argP1); + argP1 = arg; + this->IncrementArguments(newArgs,argP1,argP2); + reducible = 1; +} + +//========================================================================= +// level 0 processes parenthetical expressions +bool cmConditionEvaluator::HandleLevel0(cmArgumentList &newArgs, + std::string &errorString, + cmake::MessageType &status) +{ + int reducible; + do + { + reducible = 0; + std::list::iterator arg = newArgs.begin(); + while (arg != newArgs.end()) + { + if (*arg == "(") + { + // search for the closing paren for this opening one + std::list::iterator argClose; + argClose = arg; + argClose++; + unsigned int depth = 1; + while (argClose != newArgs.end() && depth) + { + if (*argClose == "(") + { + depth++; + } + if (*argClose == ")") + { + depth--; + } + argClose++; + } + if (depth) + { + errorString = "mismatched parenthesis in condition"; + status = cmake::FATAL_ERROR; + return false; + } + // store the reduced args in this vector + std::vector newArgs2; + + // copy to the list structure + std::list::iterator argP1 = arg; + argP1++; + for(; argP1 != argClose; argP1++) + { + newArgs2.push_back(*argP1); + } + newArgs2.pop_back(); + // now recursively invoke IsTrue to handle the values inside the + // parenthetical expression + bool value = + this->IsTrue(newArgs2, errorString, status); + if(value) + { + *arg = "1"; + } + else + { + *arg = "0"; + } + argP1 = arg; + argP1++; + // remove the now evaluated parenthetical expression + newArgs.erase(argP1,argClose); + } + ++arg; + } + } + while (reducible); + return true; +} + +//========================================================================= +// level one handles most predicates except for NOT +bool cmConditionEvaluator::HandleLevel1(cmArgumentList &newArgs, + std::string &, cmake::MessageType &) +{ + int reducible; + do + { + reducible = 0; + std::list::iterator arg = newArgs.begin(); + std::list::iterator argP1; + std::list::iterator argP2; + while (arg != newArgs.end()) + { + argP1 = arg; + this->IncrementArguments(newArgs,argP1,argP2); + // does a file exist + if (*arg == "EXISTS" && argP1 != newArgs.end()) + { + this->HandlePredicate( + cmSystemTools::FileExists((argP1)->c_str()), + reducible, arg, newArgs, argP1, argP2); + } + // does a directory with this name exist + if (*arg == "IS_DIRECTORY" && argP1 != newArgs.end()) + { + this->HandlePredicate( + cmSystemTools::FileIsDirectory((argP1)->c_str()), + reducible, arg, newArgs, argP1, argP2); + } + // does a symlink with this name exist + if (*arg == "IS_SYMLINK" && argP1 != newArgs.end()) + { + this->HandlePredicate( + cmSystemTools::FileIsSymlink((argP1)->c_str()), + reducible, arg, newArgs, argP1, argP2); + } + // is the given path an absolute path ? + if (*arg == "IS_ABSOLUTE" && argP1 != newArgs.end()) + { + this->HandlePredicate( + cmSystemTools::FileIsFullPath((argP1)->c_str()), + reducible, arg, newArgs, argP1, argP2); + } + // does a command exist + if (*arg == "COMMAND" && argP1 != newArgs.end()) + { + this->HandlePredicate( + this->Makefile.CommandExists((argP1)->c_str()), + reducible, arg, newArgs, argP1, argP2); + } + // does a policy exist + if (*arg == "POLICY" && argP1 != newArgs.end()) + { + cmPolicies::PolicyID pid; + this->HandlePredicate( + this->Makefile.GetPolicies()->GetPolicyID((argP1)->c_str(), pid), + reducible, arg, newArgs, argP1, argP2); + } + // does a target exist + if (*arg == "TARGET" && argP1 != newArgs.end()) + { + this->HandlePredicate( + this->Makefile.FindTargetToUse(*argP1)?true:false, + reducible, arg, newArgs, argP1, argP2); + } + // is a variable defined + if (*arg == "DEFINED" && argP1 != newArgs.end()) + { + size_t argP1len = argP1->size(); + bool bdef = false; + if(argP1len > 4 && argP1->substr(0, 4) == "ENV{" && + argP1->operator[](argP1len-1) == '}') + { + std::string env = argP1->substr(4, argP1len-5); + bdef = cmSystemTools::GetEnv(env.c_str())?true:false; + } + else + { + bdef = this->Makefile.IsDefinitionSet(*(argP1)); + } + this->HandlePredicate(bdef, reducible, arg, newArgs, argP1, argP2); + } + ++arg; + } + } + while (reducible); + return true; +} + +//========================================================================= +// level two handles most binary operations except for AND OR +bool cmConditionEvaluator::HandleLevel2(cmArgumentList &newArgs, + std::string &errorString, + cmake::MessageType &status) +{ + int reducible; + const char *def; + const char *def2; + do + { + reducible = 0; + std::list::iterator arg = newArgs.begin(); + std::list::iterator argP1; + std::list::iterator argP2; + while (arg != newArgs.end()) + { + argP1 = arg; + this->IncrementArguments(newArgs,argP1,argP2); + if (argP1 != newArgs.end() && argP2 != newArgs.end() && + *(argP1) == "MATCHES") + { + def = this->GetVariableOrString(*arg); + const char* rex = (argP2)->c_str(); + this->Makefile.ClearMatches(); + cmsys::RegularExpression regEntry; + if ( !regEntry.compile(rex) ) + { + cmOStringStream error; + error << "Regular expression \"" << rex << "\" cannot compile"; + errorString = error.str(); + status = cmake::FATAL_ERROR; + return false; + } + if (regEntry.find(def)) + { + this->Makefile.StoreMatches(regEntry); + *arg = "1"; + } + else + { + *arg = "0"; + } + newArgs.erase(argP2); + newArgs.erase(argP1); + argP1 = arg; + this->IncrementArguments(newArgs,argP1,argP2); + reducible = 1; + } + + if (argP1 != newArgs.end() && *arg == "MATCHES") + { + *arg = "0"; + newArgs.erase(argP1); + argP1 = arg; + this->IncrementArguments(newArgs,argP1,argP2); + reducible = 1; + } + + if (argP1 != newArgs.end() && argP2 != newArgs.end() && + (*(argP1) == "LESS" || *(argP1) == "GREATER" || + *(argP1) == "EQUAL")) + { + def = this->GetVariableOrString(*arg); + def2 = this->GetVariableOrString(*argP2); + double lhs; + double rhs; + bool result; + if(sscanf(def, "%lg", &lhs) != 1 || + sscanf(def2, "%lg", &rhs) != 1) + { + result = false; + } + else if (*(argP1) == "LESS") + { + result = (lhs < rhs); + } + else if (*(argP1) == "GREATER") + { + result = (lhs > rhs); + } + else + { + result = (lhs == rhs); + } + HandleBinaryOp(result, + reducible, arg, newArgs, argP1, argP2); + } + + if (argP1 != newArgs.end() && argP2 != newArgs.end() && + (*(argP1) == "STRLESS" || + *(argP1) == "STREQUAL" || + *(argP1) == "STRGREATER")) + { + def = this->GetVariableOrString(*arg); + def2 = this->GetVariableOrString(*argP2); + int val = strcmp(def,def2); + bool result; + if (*(argP1) == "STRLESS") + { + result = (val < 0); + } + else if (*(argP1) == "STRGREATER") + { + result = (val > 0); + } + else // strequal + { + result = (val == 0); + } + this->HandleBinaryOp(result, + reducible, arg, newArgs, argP1, argP2); + } + + if (argP1 != newArgs.end() && argP2 != newArgs.end() && + (*(argP1) == "VERSION_LESS" || *(argP1) == "VERSION_GREATER" || + *(argP1) == "VERSION_EQUAL")) + { + def = this->GetVariableOrString(*arg); + def2 = this->GetVariableOrString(*argP2); + cmSystemTools::CompareOp op = cmSystemTools::OP_EQUAL; + if(*argP1 == "VERSION_LESS") + { + op = cmSystemTools::OP_LESS; + } + else if(*argP1 == "VERSION_GREATER") + { + op = cmSystemTools::OP_GREATER; + } + bool result = cmSystemTools::VersionCompare(op, def, def2); + this->HandleBinaryOp(result, + reducible, arg, newArgs, argP1, argP2); + } + + // is file A newer than file B + if (argP1 != newArgs.end() && argP2 != newArgs.end() && + *(argP1) == "IS_NEWER_THAN") + { + int fileIsNewer=0; + bool success=cmSystemTools::FileTimeCompare(arg->c_str(), + (argP2)->c_str(), + &fileIsNewer); + this->HandleBinaryOp( + (success==false || fileIsNewer==1 || fileIsNewer==0), + reducible, arg, newArgs, argP1, argP2); + } + + ++arg; + } + } + while (reducible); + return true; +} + +//========================================================================= +// level 3 handles NOT +bool cmConditionEvaluator::HandleLevel3(cmArgumentList &newArgs, + std::string &errorString, + cmake::MessageType &status) +{ + int reducible; + do + { + reducible = 0; + std::list::iterator arg = newArgs.begin(); + std::list::iterator argP1; + std::list::iterator argP2; + while (arg != newArgs.end()) + { + argP1 = arg; + IncrementArguments(newArgs,argP1,argP2); + if (argP1 != newArgs.end() && *arg == "NOT") + { + bool rhs = this->GetBooleanValueWithAutoDereference(*argP1, + errorString, + status); + this->HandlePredicate(!rhs, reducible, arg, newArgs, argP1, argP2); + } + ++arg; + } + } + while (reducible); + return true; +} + +//========================================================================= +// level 4 handles AND OR +bool cmConditionEvaluator::HandleLevel4(cmArgumentList &newArgs, + std::string &errorString, + cmake::MessageType &status) +{ + int reducible; + bool lhs; + bool rhs; + do + { + reducible = 0; + std::list::iterator arg = newArgs.begin(); + std::list::iterator argP1; + std::list::iterator argP2; + while (arg != newArgs.end()) + { + argP1 = arg; + IncrementArguments(newArgs,argP1,argP2); + if (argP1 != newArgs.end() && *(argP1) == "AND" && + argP2 != newArgs.end()) + { + lhs = this->GetBooleanValueWithAutoDereference(*arg, + errorString, + status); + rhs = this->GetBooleanValueWithAutoDereference(*argP2, + errorString, + status); + this->HandleBinaryOp((lhs && rhs), + reducible, arg, newArgs, argP1, argP2); + } + + if (argP1 != newArgs.end() && *(argP1) == "OR" && + argP2 != newArgs.end()) + { + lhs = this->GetBooleanValueWithAutoDereference(*arg, + errorString, + status); + rhs = this->GetBooleanValueWithAutoDereference(*argP2, + errorString, + status); + this->HandleBinaryOp((lhs || rhs), + reducible, arg, newArgs, argP1, argP2); + } + ++arg; + } + } + while (reducible); + return true; +} diff --git a/Source/cmConditionEvaluator.h b/Source/cmConditionEvaluator.h new file mode 100644 index 0000000..9076988 --- /dev/null +++ b/Source/cmConditionEvaluator.h @@ -0,0 +1,90 @@ +/*============================================================================ + CMake - Cross Platform Makefile Generator + Copyright 2014 Kitware, Inc., Insight Software Consortium + + Distributed under the OSI-approved BSD License (the "License"); + see accompanying file Copyright.txt for details. + + This software is distributed WITHOUT ANY WARRANTY; without even the + implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. + See the License for more information. +============================================================================*/ +#ifndef cmConditionEvaluator_h +#define cmConditionEvaluator_h + +#include "cmCommand.h" + +class cmConditionEvaluator +{ +public: + typedef std::list cmArgumentList; + + cmConditionEvaluator(cmMakefile& makefile); + + // this is a shared function for both If and Else to determine if the + // arguments were valid, and if so, was the response true. If there is + // an error, the errorString will be set. + bool IsTrue(const std::vector &args, + std::string &errorString, + cmake::MessageType &status); + +private: + const char* GetVariableOrString( + const std::string& argument) const; + + bool IsKeyword(std::string const& keyword, + std::string& argument) const; + + bool GetBooleanValue( + std::string& arg) const; + + bool GetBooleanValueOld( + std::string const& arg, bool one) const; + + bool GetBooleanValueWithAutoDereference( + std::string &newArg, + std::string &errorString, + cmake::MessageType &status, + bool oneArg = false) const; + + void IncrementArguments( + cmArgumentList &newArgs, + cmArgumentList::iterator &argP1, + cmArgumentList::iterator &argP2) const; + + void HandlePredicate(bool value, int &reducible, + cmArgumentList::iterator &arg, + cmArgumentList &newArgs, + cmArgumentList::iterator &argP1, + cmArgumentList::iterator &argP2) const; + + void HandleBinaryOp(bool value, int &reducible, + cmArgumentList::iterator &arg, + cmArgumentList &newArgs, + cmArgumentList::iterator &argP1, + cmArgumentList::iterator &argP2); + + bool HandleLevel0(cmArgumentList &newArgs, + std::string &errorString, + cmake::MessageType &status); + + bool HandleLevel1(cmArgumentList &newArgs, + std::string &, cmake::MessageType &); + + bool HandleLevel2(cmArgumentList &newArgs, + std::string &errorString, + cmake::MessageType &status); + + bool HandleLevel3(cmArgumentList &newArgs, + std::string &errorString, + cmake::MessageType &status); + + bool HandleLevel4(cmArgumentList &newArgs, + std::string &errorString, + cmake::MessageType &status); + + cmMakefile& Makefile; + cmPolicies::PolicyStatus Policy12Status; +}; + +#endif diff --git a/Source/cmIfCommand.cxx b/Source/cmIfCommand.cxx index 1141b01..25ee25a 100644 --- a/Source/cmIfCommand.cxx +++ b/Source/cmIfCommand.cxx @@ -12,6 +12,8 @@ #include "cmIfCommand.h" #include "cmStringCommand.h" +#include "cmConditionEvaluator.h" + #include // required for atof #include #include @@ -108,9 +110,11 @@ IsFunctionBlocked(const cmListFileFunction& lff, expandedArguments); cmake::MessageType messType; - bool isTrue = - cmIfCommand::IsTrue(expandedArguments, errorString, - &mf, messType); + + cmConditionEvaluator conditionEvaluator(mf); + + bool isTrue = conditionEvaluator.IsTrue( + expandedArguments, errorString, messType); if (errorString.size()) { @@ -189,9 +193,11 @@ bool cmIfCommand this->Makefile->ExpandArguments(args, expandedArguments); cmake::MessageType status; - bool isTrue = - cmIfCommand::IsTrue(expandedArguments,errorString, - this->Makefile, status); + + cmConditionEvaluator conditionEvaluator(*(this->Makefile)); + + bool isTrue = conditionEvaluator.IsTrue( + expandedArguments, errorString, status); if (errorString.size()) { @@ -222,698 +228,3 @@ bool cmIfCommand return true; } - -namespace -{ - //========================================================================= - bool GetBooleanValue(std::string& arg, cmMakefile* mf) - { - // Check basic constants. - if (arg == "0") - { - return false; - } - if (arg == "1") - { - return true; - } - - // Check named constants. - if (cmSystemTools::IsOn(arg.c_str())) - { - return true; - } - if (cmSystemTools::IsOff(arg.c_str())) - { - return false; - } - - // Check for numbers. - if(!arg.empty()) - { - char* end; - double d = strtod(arg.c_str(), &end); - if(*end == '\0') - { - // The whole string is a number. Use C conversion to bool. - return d? true:false; - } - } - - // Check definition. - const char* def = mf->GetDefinition(arg); - return !cmSystemTools::IsOff(def); - } - - //========================================================================= - // Boolean value behavior from CMake 2.6.4 and below. - bool GetBooleanValueOld(std::string const& arg, cmMakefile* mf, bool one) - { - if(one) - { - // Old IsTrue behavior for single argument. - if(arg == "0") - { return false; } - else if(arg == "1") - { return true; } - else - { return !cmSystemTools::IsOff(mf->GetDefinition(arg)); } - } - else - { - // Old GetVariableOrNumber behavior. - const char* def = mf->GetDefinition(arg); - if(!def && atoi(arg.c_str())) - { - def = arg.c_str(); - } - return !cmSystemTools::IsOff(def); - } - } - - //========================================================================= - // returns the resulting boolean value - bool GetBooleanValueWithAutoDereference( - std::string &newArg, - cmMakefile *makefile, - std::string &errorString, - cmPolicies::PolicyStatus Policy12Status, - cmake::MessageType &status, - bool oneArg = false) - { - // Use the policy if it is set. - if (Policy12Status == cmPolicies::NEW) - { - return GetBooleanValue(newArg, makefile); - } - else if (Policy12Status == cmPolicies::OLD) - { - return GetBooleanValueOld(newArg, makefile, oneArg); - } - - // Check policy only if old and new results differ. - bool newResult = GetBooleanValue(newArg, makefile); - bool oldResult = GetBooleanValueOld(newArg, makefile, oneArg); - if(newResult != oldResult) - { - switch(Policy12Status) - { - case cmPolicies::WARN: - { - cmPolicies* policies = makefile->GetPolicies(); - errorString = "An argument named \"" + newArg - + "\" appears in a conditional statement. " - + policies->GetPolicyWarning(cmPolicies::CMP0012); - status = cmake::AUTHOR_WARNING; - } - case cmPolicies::OLD: - return oldResult; - case cmPolicies::REQUIRED_IF_USED: - case cmPolicies::REQUIRED_ALWAYS: - { - cmPolicies* policies = makefile->GetPolicies(); - errorString = "An argument named \"" + newArg - + "\" appears in a conditional statement. " - + policies->GetRequiredPolicyError(cmPolicies::CMP0012); - status = cmake::FATAL_ERROR; - } - case cmPolicies::NEW: - break; - } - } - return newResult; - } - - //========================================================================= - void IncrementArguments(std::list &newArgs, - std::list::iterator &argP1, - std::list::iterator &argP2) - { - if (argP1 != newArgs.end()) - { - argP1++; - argP2 = argP1; - if (argP1 != newArgs.end()) - { - argP2++; - } - } - } - - //========================================================================= - // helper function to reduce code duplication - void HandlePredicate(bool value, int &reducible, - std::list::iterator &arg, - std::list &newArgs, - std::list::iterator &argP1, - std::list::iterator &argP2) - { - if(value) - { - *arg = "1"; - } - else - { - *arg = "0"; - } - newArgs.erase(argP1); - argP1 = arg; - IncrementArguments(newArgs,argP1,argP2); - reducible = 1; - } - - //========================================================================= - // helper function to reduce code duplication - void HandleBinaryOp(bool value, int &reducible, - std::list::iterator &arg, - std::list &newArgs, - std::list::iterator &argP1, - std::list::iterator &argP2) - { - if(value) - { - *arg = "1"; - } - else - { - *arg = "0"; - } - newArgs.erase(argP2); - newArgs.erase(argP1); - argP1 = arg; - IncrementArguments(newArgs,argP1,argP2); - reducible = 1; - } - - //========================================================================= - // level 0 processes parenthetical expressions - bool HandleLevel0(std::list &newArgs, - cmMakefile *makefile, - std::string &errorString, - cmake::MessageType &status) - { - int reducible; - do - { - reducible = 0; - std::list::iterator arg = newArgs.begin(); - while (arg != newArgs.end()) - { - if (*arg == "(") - { - // search for the closing paren for this opening one - std::list::iterator argClose; - argClose = arg; - argClose++; - unsigned int depth = 1; - while (argClose != newArgs.end() && depth) - { - if (*argClose == "(") - { - depth++; - } - if (*argClose == ")") - { - depth--; - } - argClose++; - } - if (depth) - { - errorString = "mismatched parenthesis in condition"; - status = cmake::FATAL_ERROR; - return false; - } - // store the reduced args in this vector - std::vector newArgs2; - - // copy to the list structure - std::list::iterator argP1 = arg; - argP1++; - for(; argP1 != argClose; argP1++) - { - newArgs2.push_back(*argP1); - } - newArgs2.pop_back(); - // now recursively invoke IsTrue to handle the values inside the - // parenthetical expression - bool value = - cmIfCommand::IsTrue(newArgs2, errorString, makefile, status); - if(value) - { - *arg = "1"; - } - else - { - *arg = "0"; - } - argP1 = arg; - argP1++; - // remove the now evaluated parenthetical expression - newArgs.erase(argP1,argClose); - } - ++arg; - } - } - while (reducible); - return true; - } - - //========================================================================= - // level one handles most predicates except for NOT - bool HandleLevel1(std::list &newArgs, - cmMakefile *makefile, - std::string &, cmake::MessageType &) - { - int reducible; - do - { - reducible = 0; - std::list::iterator arg = newArgs.begin(); - std::list::iterator argP1; - std::list::iterator argP2; - while (arg != newArgs.end()) - { - argP1 = arg; - IncrementArguments(newArgs,argP1,argP2); - // does a file exist - if (*arg == "EXISTS" && argP1 != newArgs.end()) - { - HandlePredicate( - cmSystemTools::FileExists((argP1)->c_str()), - reducible, arg, newArgs, argP1, argP2); - } - // does a directory with this name exist - if (*arg == "IS_DIRECTORY" && argP1 != newArgs.end()) - { - HandlePredicate( - cmSystemTools::FileIsDirectory((argP1)->c_str()), - reducible, arg, newArgs, argP1, argP2); - } - // does a symlink with this name exist - if (*arg == "IS_SYMLINK" && argP1 != newArgs.end()) - { - HandlePredicate( - cmSystemTools::FileIsSymlink((argP1)->c_str()), - reducible, arg, newArgs, argP1, argP2); - } - // is the given path an absolute path ? - if (*arg == "IS_ABSOLUTE" && argP1 != newArgs.end()) - { - HandlePredicate( - cmSystemTools::FileIsFullPath((argP1)->c_str()), - reducible, arg, newArgs, argP1, argP2); - } - // does a command exist - if (*arg == "COMMAND" && argP1 != newArgs.end()) - { - HandlePredicate( - makefile->CommandExists((argP1)->c_str()), - reducible, arg, newArgs, argP1, argP2); - } - // does a policy exist - if (*arg == "POLICY" && argP1 != newArgs.end()) - { - cmPolicies::PolicyID pid; - HandlePredicate( - makefile->GetPolicies()->GetPolicyID((argP1)->c_str(), pid), - reducible, arg, newArgs, argP1, argP2); - } - // does a target exist - if (*arg == "TARGET" && argP1 != newArgs.end()) - { - HandlePredicate( - makefile->FindTargetToUse(*argP1)?true:false, - reducible, arg, newArgs, argP1, argP2); - } - // is a variable defined - if (*arg == "DEFINED" && argP1 != newArgs.end()) - { - size_t argP1len = argP1->size(); - bool bdef = false; - if(argP1len > 4 && argP1->substr(0, 4) == "ENV{" && - argP1->operator[](argP1len-1) == '}') - { - std::string env = argP1->substr(4, argP1len-5); - bdef = cmSystemTools::GetEnv(env.c_str())?true:false; - } - else - { - bdef = makefile->IsDefinitionSet(*(argP1)); - } - HandlePredicate(bdef, reducible, arg, newArgs, argP1, argP2); - } - ++arg; - } - } - while (reducible); - return true; - } - - //========================================================================= - // level two handles most binary operations except for AND OR - bool HandleLevel2(std::list &newArgs, - cmMakefile *makefile, - std::string &errorString, - cmake::MessageType &status) - { - int reducible; - const char *def; - const char *def2; - do - { - reducible = 0; - std::list::iterator arg = newArgs.begin(); - std::list::iterator argP1; - std::list::iterator argP2; - while (arg != newArgs.end()) - { - argP1 = arg; - IncrementArguments(newArgs,argP1,argP2); - if (argP1 != newArgs.end() && argP2 != newArgs.end() && - *(argP1) == "MATCHES") - { - def = cmIfCommand::GetVariableOrString(*arg, makefile); - const char* rex = (argP2)->c_str(); - makefile->ClearMatches(); - cmsys::RegularExpression regEntry; - if ( !regEntry.compile(rex) ) - { - cmOStringStream error; - error << "Regular expression \"" << rex << "\" cannot compile"; - errorString = error.str(); - status = cmake::FATAL_ERROR; - return false; - } - if (regEntry.find(def)) - { - makefile->StoreMatches(regEntry); - *arg = "1"; - } - else - { - *arg = "0"; - } - newArgs.erase(argP2); - newArgs.erase(argP1); - argP1 = arg; - IncrementArguments(newArgs,argP1,argP2); - reducible = 1; - } - - if (argP1 != newArgs.end() && *arg == "MATCHES") - { - *arg = "0"; - newArgs.erase(argP1); - argP1 = arg; - IncrementArguments(newArgs,argP1,argP2); - reducible = 1; - } - - if (argP1 != newArgs.end() && argP2 != newArgs.end() && - (*(argP1) == "LESS" || *(argP1) == "GREATER" || - *(argP1) == "EQUAL")) - { - def = cmIfCommand::GetVariableOrString(*arg, makefile); - def2 = cmIfCommand::GetVariableOrString(*argP2, makefile); - double lhs; - double rhs; - bool result; - if(sscanf(def, "%lg", &lhs) != 1 || - sscanf(def2, "%lg", &rhs) != 1) - { - result = false; - } - else if (*(argP1) == "LESS") - { - result = (lhs < rhs); - } - else if (*(argP1) == "GREATER") - { - result = (lhs > rhs); - } - else - { - result = (lhs == rhs); - } - HandleBinaryOp(result, - reducible, arg, newArgs, argP1, argP2); - } - - if (argP1 != newArgs.end() && argP2 != newArgs.end() && - (*(argP1) == "STRLESS" || - *(argP1) == "STREQUAL" || - *(argP1) == "STRGREATER")) - { - def = cmIfCommand::GetVariableOrString(*arg, makefile); - def2 = cmIfCommand::GetVariableOrString(*argP2, makefile); - int val = strcmp(def,def2); - bool result; - if (*(argP1) == "STRLESS") - { - result = (val < 0); - } - else if (*(argP1) == "STRGREATER") - { - result = (val > 0); - } - else // strequal - { - result = (val == 0); - } - HandleBinaryOp(result, - reducible, arg, newArgs, argP1, argP2); - } - - if (argP1 != newArgs.end() && argP2 != newArgs.end() && - (*(argP1) == "VERSION_LESS" || *(argP1) == "VERSION_GREATER" || - *(argP1) == "VERSION_EQUAL")) - { - def = cmIfCommand::GetVariableOrString(*arg, makefile); - def2 = cmIfCommand::GetVariableOrString(*argP2, makefile); - cmSystemTools::CompareOp op = cmSystemTools::OP_EQUAL; - if(*argP1 == "VERSION_LESS") - { - op = cmSystemTools::OP_LESS; - } - else if(*argP1 == "VERSION_GREATER") - { - op = cmSystemTools::OP_GREATER; - } - bool result = cmSystemTools::VersionCompare(op, def, def2); - HandleBinaryOp(result, - reducible, arg, newArgs, argP1, argP2); - } - - // is file A newer than file B - if (argP1 != newArgs.end() && argP2 != newArgs.end() && - *(argP1) == "IS_NEWER_THAN") - { - int fileIsNewer=0; - bool success=cmSystemTools::FileTimeCompare(arg->c_str(), - (argP2)->c_str(), - &fileIsNewer); - HandleBinaryOp( - (success==false || fileIsNewer==1 || fileIsNewer==0), - reducible, arg, newArgs, argP1, argP2); - } - - ++arg; - } - } - while (reducible); - return true; - } - - //========================================================================= - // level 3 handles NOT - bool HandleLevel3(std::list &newArgs, - cmMakefile *makefile, - std::string &errorString, - cmPolicies::PolicyStatus Policy12Status, - cmake::MessageType &status) - { - int reducible; - do - { - reducible = 0; - std::list::iterator arg = newArgs.begin(); - std::list::iterator argP1; - std::list::iterator argP2; - while (arg != newArgs.end()) - { - argP1 = arg; - IncrementArguments(newArgs,argP1,argP2); - if (argP1 != newArgs.end() && *arg == "NOT") - { - bool rhs = GetBooleanValueWithAutoDereference(*argP1, makefile, - errorString, - Policy12Status, - status); - HandlePredicate(!rhs, reducible, arg, newArgs, argP1, argP2); - } - ++arg; - } - } - while (reducible); - return true; - } - - //========================================================================= - // level 4 handles AND OR - bool HandleLevel4(std::list &newArgs, - cmMakefile *makefile, - std::string &errorString, - cmPolicies::PolicyStatus Policy12Status, - cmake::MessageType &status) - { - int reducible; - bool lhs; - bool rhs; - do - { - reducible = 0; - std::list::iterator arg = newArgs.begin(); - std::list::iterator argP1; - std::list::iterator argP2; - while (arg != newArgs.end()) - { - argP1 = arg; - IncrementArguments(newArgs,argP1,argP2); - if (argP1 != newArgs.end() && *(argP1) == "AND" && - argP2 != newArgs.end()) - { - lhs = GetBooleanValueWithAutoDereference(*arg, makefile, - errorString, - Policy12Status, - status); - rhs = GetBooleanValueWithAutoDereference(*argP2, makefile, - errorString, - Policy12Status, - status); - HandleBinaryOp((lhs && rhs), - reducible, arg, newArgs, argP1, argP2); - } - - if (argP1 != newArgs.end() && *(argP1) == "OR" && - argP2 != newArgs.end()) - { - lhs = GetBooleanValueWithAutoDereference(*arg, makefile, - errorString, - Policy12Status, - status); - rhs = GetBooleanValueWithAutoDereference(*argP2, makefile, - errorString, - Policy12Status, - status); - HandleBinaryOp((lhs || rhs), - reducible, arg, newArgs, argP1, argP2); - } - ++arg; - } - } - while (reducible); - return true; - } -} - - -//========================================================================= -// order of operations, -// 1. ( ) -- parenthetical groups -// 2. IS_DIRECTORY EXISTS COMMAND DEFINED etc predicates -// 3. MATCHES LESS GREATER EQUAL STRLESS STRGREATER STREQUAL etc binary ops -// 4. NOT -// 5. AND OR -// -// There is an issue on whether the arguments should be values of references, -// for example IF (FOO AND BAR) should that compare the strings FOO and BAR -// or should it really do IF (${FOO} AND ${BAR}) Currently IS_DIRECTORY -// EXISTS COMMAND and DEFINED all take values. EQUAL, LESS and GREATER can -// take numeric values or variable names. STRLESS and STRGREATER take -// variable names but if the variable name is not found it will use the name -// directly. AND OR take variables or the values 0 or 1. - - -bool cmIfCommand::IsTrue(const std::vector &args, - std::string &errorString, cmMakefile *makefile, - cmake::MessageType &status) -{ - errorString = ""; - - // handle empty invocation - if (args.size() < 1) - { - return false; - } - - // store the reduced args in this vector - std::list newArgs; - - // copy to the list structure - for(unsigned int i = 0; i < args.size(); ++i) - { - newArgs.push_back(args[i]); - } - - // now loop through the arguments and see if we can reduce any of them - // we do this multiple times. Once for each level of precedence - // parens - if (!HandleLevel0(newArgs, makefile, errorString, status)) - { - return false; - } - //predicates - if (!HandleLevel1(newArgs, makefile, errorString, status)) - { - return false; - } - // binary ops - if (!HandleLevel2(newArgs, makefile, errorString, status)) - { - return false; - } - - // used to store the value of policy CMP0012 for performance - cmPolicies::PolicyStatus Policy12Status = - makefile->GetPolicyStatus(cmPolicies::CMP0012); - - // NOT - if (!HandleLevel3(newArgs, makefile, errorString, - Policy12Status, status)) - { - return false; - } - // AND OR - if (!HandleLevel4(newArgs, makefile, errorString, - Policy12Status, status)) - { - return false; - } - - // now at the end there should only be one argument left - if (newArgs.size() != 1) - { - errorString = "Unknown arguments specified"; - status = cmake::FATAL_ERROR; - return false; - } - - return GetBooleanValueWithAutoDereference(*(newArgs.begin()), - makefile, - errorString, - Policy12Status, - status, true); -} - -//========================================================================= -const char* cmIfCommand::GetVariableOrString(const std::string& str, - const cmMakefile* mf) -{ - const char* def = mf->GetDefinition(str); - if(!def) - { - def = str.c_str(); - } - return def; -} diff --git a/Source/cmIfCommand.h b/Source/cmIfCommand.h index 814c052..ea2a68f 100644 --- a/Source/cmIfCommand.h +++ b/Source/cmIfCommand.h @@ -70,18 +70,6 @@ public: */ virtual bool IsScriptable() const { return true; } - // this is a shared function for both If and Else to determine if the - // arguments were valid, and if so, was the response true. If there is - // an error, the errorString will be set. - static bool IsTrue(const std::vector &args, - std::string &errorString, cmMakefile *mf, - cmake::MessageType &status); - - // Get a definition from the makefile. If it doesn't exist, - // return the original string. - static const char* GetVariableOrString(const std::string& str, - const cmMakefile* mf); - cmTypeMacro(cmIfCommand, cmCommand); }; diff --git a/Source/cmWhileCommand.cxx b/Source/cmWhileCommand.cxx index 7d2eead..5565b8a 100644 --- a/Source/cmWhileCommand.cxx +++ b/Source/cmWhileCommand.cxx @@ -10,7 +10,7 @@ See the License for more information. ============================================================================*/ #include "cmWhileCommand.h" -#include "cmIfCommand.h" +#include "cmConditionEvaluator.h" bool cmWhileFunctionBlocker:: IsFunctionBlocked(const cmListFileFunction& lff, cmMakefile &mf, @@ -37,9 +37,11 @@ IsFunctionBlocked(const cmListFileFunction& lff, cmMakefile &mf, std::vector expandedArguments; mf.ExpandArguments(this->Args, expandedArguments); cmake::MessageType messageType; - bool isTrue = - cmIfCommand::IsTrue(expandedArguments,errorString, - &mf, messageType); + + cmConditionEvaluator conditionEvaluator(mf); + + bool isTrue = conditionEvaluator.IsTrue( + expandedArguments, errorString, messageType); while (isTrue) { @@ -86,9 +88,8 @@ IsFunctionBlocked(const cmListFileFunction& lff, cmMakefile &mf, } expandedArguments.clear(); mf.ExpandArguments(this->Args, expandedArguments); - isTrue = - cmIfCommand::IsTrue(expandedArguments,errorString, - &mf, messageType); + isTrue = conditionEvaluator.IsTrue( + expandedArguments, errorString, messageType); } return true; } -- cgit v0.12 From 188a1f236e1594fc46163fbad5e062e3978c1e5a Mon Sep 17 00:00:00 2001 From: Nils Gladitz Date: Thu, 4 Sep 2014 20:21:28 +0200 Subject: If: Introduce policy CMP0054 - don't dereference quoted variables in if() --- Help/command/if.rst | 6 + Help/manual/cmake-policies.7.rst | 1 + Help/policy/CMP0054.rst | 18 ++ Help/release/dev/if-sanity.rst | 6 + Source/cmBootstrapCommands2.cxx | 1 + Source/cmConditionEvaluator.cxx | 236 ++++++++++++++------- Source/cmConditionEvaluator.h | 20 +- Source/cmExpandedCommandArgument.cxx | 51 +++++ Source/cmExpandedCommandArgument.h | 45 ++++ Source/cmIfCommand.cxx | 10 +- Source/cmIfCommand.h | 4 + Source/cmMakefile.cxx | 61 ++++++ Source/cmMakefile.h | 35 ++- Source/cmPolicies.cxx | 5 + Source/cmPolicies.h | 2 + Source/cmWhileCommand.cxx | 2 +- Tests/RunCMake/CMP0054/CMP0054-NEW-stderr.txt | 1 + Tests/RunCMake/CMP0054/CMP0054-NEW.cmake | 47 ++++ Tests/RunCMake/CMP0054/CMP0054-OLD-stderr.txt | 1 + Tests/RunCMake/CMP0054/CMP0054-OLD.cmake | 47 ++++ Tests/RunCMake/CMP0054/CMP0054-WARN-stderr.txt | 11 + Tests/RunCMake/CMP0054/CMP0054-WARN.cmake | 5 + .../CMP0054/CMP0054-duplicate-warnings-stderr.txt | 24 +++ .../CMP0054/CMP0054-duplicate-warnings.cmake | 12 ++ .../CMP0054/CMP0054-keywords-NEW-result.txt | 1 + .../CMP0054/CMP0054-keywords-NEW-stderr.txt | 8 + Tests/RunCMake/CMP0054/CMP0054-keywords-NEW.cmake | 25 +++ .../CMP0054/CMP0054-keywords-OLD-stderr.txt | 1 + Tests/RunCMake/CMP0054/CMP0054-keywords-OLD.cmake | 9 + .../CMP0054/CMP0054-keywords-WARN-stderr.txt | 12 ++ Tests/RunCMake/CMP0054/CMP0054-keywords-WARN.cmake | 3 + .../CMP0054-policy-command-scope-stderr.txt | 1 + .../CMP0054/CMP0054-policy-command-scope.cmake | 53 +++++ .../CMP0054-policy-foreach-scope-stderr.txt | 1 + .../CMP0054/CMP0054-policy-foreach-scope.cmake | 49 +++++ .../CMP0054/CMP0054-policy-nested-if-stderr.txt | 1 + .../CMP0054/CMP0054-policy-nested-if.cmake | 41 ++++ .../CMP0054/CMP0054-policy-while-scope-stderr.txt | 1 + .../CMP0054/CMP0054-policy-while-scope.cmake | 65 ++++++ Tests/RunCMake/CMP0054/CMakeLists.txt | 3 + Tests/RunCMake/CMP0054/RunCMakeTest.cmake | 13 ++ Tests/RunCMake/CMakeLists.txt | 1 + 42 files changed, 846 insertions(+), 93 deletions(-) create mode 100644 Help/policy/CMP0054.rst create mode 100644 Help/release/dev/if-sanity.rst create mode 100644 Source/cmExpandedCommandArgument.cxx create mode 100644 Source/cmExpandedCommandArgument.h create mode 100644 Tests/RunCMake/CMP0054/CMP0054-NEW-stderr.txt create mode 100644 Tests/RunCMake/CMP0054/CMP0054-NEW.cmake create mode 100644 Tests/RunCMake/CMP0054/CMP0054-OLD-stderr.txt create mode 100644 Tests/RunCMake/CMP0054/CMP0054-OLD.cmake create mode 100644 Tests/RunCMake/CMP0054/CMP0054-WARN-stderr.txt create mode 100644 Tests/RunCMake/CMP0054/CMP0054-WARN.cmake create mode 100644 Tests/RunCMake/CMP0054/CMP0054-duplicate-warnings-stderr.txt create mode 100644 Tests/RunCMake/CMP0054/CMP0054-duplicate-warnings.cmake create mode 100644 Tests/RunCMake/CMP0054/CMP0054-keywords-NEW-result.txt create mode 100644 Tests/RunCMake/CMP0054/CMP0054-keywords-NEW-stderr.txt create mode 100644 Tests/RunCMake/CMP0054/CMP0054-keywords-NEW.cmake create mode 100644 Tests/RunCMake/CMP0054/CMP0054-keywords-OLD-stderr.txt create mode 100644 Tests/RunCMake/CMP0054/CMP0054-keywords-OLD.cmake create mode 100644 Tests/RunCMake/CMP0054/CMP0054-keywords-WARN-stderr.txt create mode 100644 Tests/RunCMake/CMP0054/CMP0054-keywords-WARN.cmake create mode 100644 Tests/RunCMake/CMP0054/CMP0054-policy-command-scope-stderr.txt create mode 100644 Tests/RunCMake/CMP0054/CMP0054-policy-command-scope.cmake create mode 100644 Tests/RunCMake/CMP0054/CMP0054-policy-foreach-scope-stderr.txt create mode 100644 Tests/RunCMake/CMP0054/CMP0054-policy-foreach-scope.cmake create mode 100644 Tests/RunCMake/CMP0054/CMP0054-policy-nested-if-stderr.txt create mode 100644 Tests/RunCMake/CMP0054/CMP0054-policy-nested-if.cmake create mode 100644 Tests/RunCMake/CMP0054/CMP0054-policy-while-scope-stderr.txt create mode 100644 Tests/RunCMake/CMP0054/CMP0054-policy-while-scope.cmake create mode 100644 Tests/RunCMake/CMP0054/CMakeLists.txt create mode 100644 Tests/RunCMake/CMP0054/RunCMakeTest.cmake diff --git a/Help/command/if.rst b/Help/command/if.rst index a45b995..79e5d21 100644 --- a/Help/command/if.rst +++ b/Help/command/if.rst @@ -199,3 +199,9 @@ above-documented signature accepts ````: * The left and right hand arguments to ``AND`` and ``OR`` are independently tested to see if they are boolean constants, if so they are used as such, otherwise they are assumed to be variables and are dereferenced. + +To prevent ambiguity, potential variable or keyword names can be +specified in a :ref:`Quoted Argument` or a :ref:`Bracket Argument`. +A quoted or bracketed variable or keyword will be interpreted as a +string and not dereferenced or interpreted. +See policy :policy:`CMP0054`. diff --git a/Help/manual/cmake-policies.7.rst b/Help/manual/cmake-policies.7.rst index 136cf5c..f1717a0 100644 --- a/Help/manual/cmake-policies.7.rst +++ b/Help/manual/cmake-policies.7.rst @@ -105,3 +105,4 @@ All Policies /policy/CMP0051 /policy/CMP0052 /policy/CMP0053 + /policy/CMP0054 diff --git a/Help/policy/CMP0054.rst b/Help/policy/CMP0054.rst new file mode 100644 index 0000000..dffee5e --- /dev/null +++ b/Help/policy/CMP0054.rst @@ -0,0 +1,18 @@ +CMP0054 +------- + +Only interpret :command:`if` arguments as variables or keywords when unquoted. + +CMake 3.1 and above no longer dereference variables or interpret keywords +in an :command:`if` command argument when it is a :ref:`Quoted Argument` +or a :ref:`Bracket Argument`. + +The ``OLD`` behavior for this policy is to dereference variables and +interpret keywords even if they are quoted or bracketed. +The ``NEW`` behavior is to not dereference variables or interpret keywords +that have been quoted or bracketed. + +This policy was introduced in CMake version 3.1. +CMake version |release| warns when the policy is not set and uses +``OLD`` behavior. Use the :command:`cmake_policy` command to set +it to ``OLD`` or ``NEW`` explicitly. diff --git a/Help/release/dev/if-sanity.rst b/Help/release/dev/if-sanity.rst new file mode 100644 index 0000000..6645bc4 --- /dev/null +++ b/Help/release/dev/if-sanity.rst @@ -0,0 +1,6 @@ +if-sanity +--------- + +* The :command:`if` command no longer automatically dereferences + variables named in quoted or bracket arguments. See policy + :policy:`CMP0054`. diff --git a/Source/cmBootstrapCommands2.cxx b/Source/cmBootstrapCommands2.cxx index 91c189f..5675295 100644 --- a/Source/cmBootstrapCommands2.cxx +++ b/Source/cmBootstrapCommands2.cxx @@ -15,6 +15,7 @@ // like to have CMake to build CMake. #include "cmCommands.h" #include "cmConditionEvaluator.cxx" +#include "cmExpandedCommandArgument.cxx" #include "cmGeneratorExpressionEvaluationFile.cxx" #include "cmGetCMakePropertyCommand.cxx" #include "cmGetDirectoryPropertyCommand.cxx" diff --git a/Source/cmConditionEvaluator.cxx b/Source/cmConditionEvaluator.cxx index 869d3c5..aba26de 100644 --- a/Source/cmConditionEvaluator.cxx +++ b/Source/cmConditionEvaluator.cxx @@ -14,7 +14,8 @@ cmConditionEvaluator::cmConditionEvaluator(cmMakefile& makefile): Makefile(makefile), - Policy12Status(makefile.GetPolicyStatus(cmPolicies::CMP0012)) + Policy12Status(makefile.GetPolicyStatus(cmPolicies::CMP0012)), + Policy54Status(makefile.GetPolicyStatus(cmPolicies::CMP0054)) { } @@ -36,7 +37,7 @@ cmConditionEvaluator::cmConditionEvaluator(cmMakefile& makefile): // directly. AND OR take variables or the values 0 or 1. bool cmConditionEvaluator::IsTrue( - const std::vector &args, + const std::vector &args, std::string &errorString, cmake::MessageType &status) { @@ -99,22 +100,93 @@ bool cmConditionEvaluator::IsTrue( } //========================================================================= +const char* cmConditionEvaluator::GetDefinitionIfUnquoted( + cmExpandedCommandArgument const& argument) const +{ + if((this->Policy54Status != cmPolicies::WARN && + this->Policy54Status != cmPolicies::OLD) && + argument.WasQuoted()) + { + return 0; + } + + const char* def = this->Makefile.GetDefinition(argument.GetValue()); + + if(def && argument.WasQuoted() && this->Policy54Status == cmPolicies::WARN) + { + bool hasBeenReported = this->Makefile.HasCMP0054AlreadyBeenReported( + this->Makefile.GetBacktrace()[0]); + + if(!hasBeenReported) + { + cmOStringStream e; + e << (this->Makefile.GetPolicies()->GetPolicyWarning( + cmPolicies::CMP0054)) << "\n"; + e << "Quoted variables like \"" << argument.GetValue() << + "\" will no longer be dereferenced " + "when the policy is set to NEW. " + "Since the policy is not set the OLD behavior will be used."; + + this->Makefile.IssueMessage(cmake::AUTHOR_WARNING, e.str()); + } + } + + return def; +} + +//========================================================================= const char* cmConditionEvaluator::GetVariableOrString( - const std::string& str) const + const cmExpandedCommandArgument& argument) const { - const char* def = this->Makefile.GetDefinition(str); + const char* def = this->GetDefinitionIfUnquoted(argument); if(!def) { - def = str.c_str(); + def = argument.c_str(); } return def; } //========================================================================= +bool cmConditionEvaluator::IsKeyword(std::string const& keyword, + cmExpandedCommandArgument& argument) const +{ + if((this->Policy54Status != cmPolicies::WARN && + this->Policy54Status != cmPolicies::OLD) && + argument.WasQuoted()) + { + return false; + } + + bool isKeyword = argument.GetValue() == keyword; + + if(isKeyword && argument.WasQuoted() && + this->Policy54Status == cmPolicies::WARN) + { + bool hasBeenReported = this->Makefile.HasCMP0054AlreadyBeenReported( + this->Makefile.GetBacktrace()[0]); + + if(!hasBeenReported) + { + cmOStringStream e; + e << (this->Makefile.GetPolicies()->GetPolicyWarning( + cmPolicies::CMP0054)) << "\n"; + e << "Quoted keywords like \"" << argument.GetValue() << + "\" will no longer be interpreted as keywords " + "when the policy is set to NEW. " + "Since the policy is not set the OLD behavior will be used."; + + this->Makefile.IssueMessage(cmake::AUTHOR_WARNING, e.str()); + } + } + + return isKeyword; +} + +//========================================================================= bool cmConditionEvaluator::GetBooleanValue( - std::string& arg) const + cmExpandedCommandArgument& arg) const { // Check basic constants. if (arg == "0") @@ -149,14 +221,14 @@ bool cmConditionEvaluator::GetBooleanValue( } // Check definition. - const char* def = this->Makefile.GetDefinition(arg); + const char* def = this->GetDefinitionIfUnquoted(arg); return !cmSystemTools::IsOff(def); } //========================================================================= // Boolean value behavior from CMake 2.6.4 and below. bool cmConditionEvaluator::GetBooleanValueOld( - std::string const& arg, bool one) const + cmExpandedCommandArgument const& arg, bool one) const { if(one) { @@ -166,12 +238,15 @@ bool cmConditionEvaluator::GetBooleanValueOld( else if(arg == "1") { return true; } else - { return !cmSystemTools::IsOff(this->Makefile.GetDefinition(arg)); } + { + const char* def = this->GetDefinitionIfUnquoted(arg); + return !cmSystemTools::IsOff(def); + } } else { // Old GetVariableOrNumber behavior. - const char* def = this->Makefile.GetDefinition(arg); + const char* def = this->GetDefinitionIfUnquoted(arg); if(!def && atoi(arg.c_str())) { def = arg.c_str(); @@ -183,7 +258,7 @@ bool cmConditionEvaluator::GetBooleanValueOld( //========================================================================= // returns the resulting boolean value bool cmConditionEvaluator::GetBooleanValueWithAutoDereference( - std::string &newArg, + cmExpandedCommandArgument &newArg, std::string &errorString, cmake::MessageType &status, bool oneArg) const @@ -208,7 +283,7 @@ bool cmConditionEvaluator::GetBooleanValueWithAutoDereference( case cmPolicies::WARN: { cmPolicies* policies = this->Makefile.GetPolicies(); - errorString = "An argument named \"" + newArg + errorString = "An argument named \"" + newArg.GetValue() + "\" appears in a conditional statement. " + policies->GetPolicyWarning(cmPolicies::CMP0012); status = cmake::AUTHOR_WARNING; @@ -219,7 +294,7 @@ bool cmConditionEvaluator::GetBooleanValueWithAutoDereference( case cmPolicies::REQUIRED_ALWAYS: { cmPolicies* policies = this->Makefile.GetPolicies(); - errorString = "An argument named \"" + newArg + errorString = "An argument named \"" + newArg.GetValue() + "\" appears in a conditional statement. " + policies->GetRequiredPolicyError(cmPolicies::CMP0012); status = cmake::FATAL_ERROR; @@ -257,11 +332,11 @@ void cmConditionEvaluator::HandlePredicate(bool value, int &reducible, { if(value) { - *arg = "1"; + *arg = cmExpandedCommandArgument("1", true); } else { - *arg = "0"; + *arg = cmExpandedCommandArgument("0", true); } newArgs.erase(argP1); argP1 = arg; @@ -279,11 +354,11 @@ void cmConditionEvaluator::HandleBinaryOp(bool value, int &reducible, { if(value) { - *arg = "1"; + *arg = cmExpandedCommandArgument("1", true); } else { - *arg = "0"; + *arg = cmExpandedCommandArgument("0", true); } newArgs.erase(argP2); newArgs.erase(argP1); @@ -302,23 +377,23 @@ bool cmConditionEvaluator::HandleLevel0(cmArgumentList &newArgs, do { reducible = 0; - std::list::iterator arg = newArgs.begin(); + cmArgumentList::iterator arg = newArgs.begin(); while (arg != newArgs.end()) { - if (*arg == "(") + if (IsKeyword("(", *arg)) { // search for the closing paren for this opening one - std::list::iterator argClose; + cmArgumentList::iterator argClose; argClose = arg; argClose++; unsigned int depth = 1; while (argClose != newArgs.end() && depth) { - if (*argClose == "(") + if (this->IsKeyword("(", *argClose)) { depth++; } - if (*argClose == ")") + if (this->IsKeyword(")", *argClose)) { depth--; } @@ -331,10 +406,10 @@ bool cmConditionEvaluator::HandleLevel0(cmArgumentList &newArgs, return false; } // store the reduced args in this vector - std::vector newArgs2; + std::vector newArgs2; // copy to the list structure - std::list::iterator argP1 = arg; + cmArgumentList::iterator argP1 = arg; argP1++; for(; argP1 != argClose; argP1++) { @@ -347,11 +422,11 @@ bool cmConditionEvaluator::HandleLevel0(cmArgumentList &newArgs, this->IsTrue(newArgs2, errorString, status); if(value) { - *arg = "1"; + *arg = cmExpandedCommandArgument("1", true); } else { - *arg = "0"; + *arg = cmExpandedCommandArgument("0", true); } argP1 = arg; argP1++; @@ -374,77 +449,78 @@ bool cmConditionEvaluator::HandleLevel1(cmArgumentList &newArgs, do { reducible = 0; - std::list::iterator arg = newArgs.begin(); - std::list::iterator argP1; - std::list::iterator argP2; + cmArgumentList::iterator arg = newArgs.begin(); + cmArgumentList::iterator argP1; + cmArgumentList::iterator argP2; while (arg != newArgs.end()) { argP1 = arg; this->IncrementArguments(newArgs,argP1,argP2); // does a file exist - if (*arg == "EXISTS" && argP1 != newArgs.end()) + if (this->IsKeyword("EXISTS", *arg) && argP1 != newArgs.end()) { this->HandlePredicate( - cmSystemTools::FileExists((argP1)->c_str()), + cmSystemTools::FileExists(argP1->c_str()), reducible, arg, newArgs, argP1, argP2); } // does a directory with this name exist - if (*arg == "IS_DIRECTORY" && argP1 != newArgs.end()) + if (this->IsKeyword("IS_DIRECTORY", *arg) && argP1 != newArgs.end()) { this->HandlePredicate( - cmSystemTools::FileIsDirectory((argP1)->c_str()), + cmSystemTools::FileIsDirectory(argP1->c_str()), reducible, arg, newArgs, argP1, argP2); } // does a symlink with this name exist - if (*arg == "IS_SYMLINK" && argP1 != newArgs.end()) + if (this->IsKeyword("IS_SYMLINK", *arg) && argP1 != newArgs.end()) { this->HandlePredicate( - cmSystemTools::FileIsSymlink((argP1)->c_str()), + cmSystemTools::FileIsSymlink(argP1->c_str()), reducible, arg, newArgs, argP1, argP2); } // is the given path an absolute path ? - if (*arg == "IS_ABSOLUTE" && argP1 != newArgs.end()) + if (this->IsKeyword("IS_ABSOLUTE", *arg) && argP1 != newArgs.end()) { this->HandlePredicate( - cmSystemTools::FileIsFullPath((argP1)->c_str()), + cmSystemTools::FileIsFullPath(argP1->c_str()), reducible, arg, newArgs, argP1, argP2); } // does a command exist - if (*arg == "COMMAND" && argP1 != newArgs.end()) + if (this->IsKeyword("COMMAND", *arg) && argP1 != newArgs.end()) { this->HandlePredicate( - this->Makefile.CommandExists((argP1)->c_str()), + this->Makefile.CommandExists(argP1->c_str()), reducible, arg, newArgs, argP1, argP2); } // does a policy exist - if (*arg == "POLICY" && argP1 != newArgs.end()) + if (this->IsKeyword("POLICY", *arg) && argP1 != newArgs.end()) { cmPolicies::PolicyID pid; this->HandlePredicate( - this->Makefile.GetPolicies()->GetPolicyID((argP1)->c_str(), pid), - reducible, arg, newArgs, argP1, argP2); + this->Makefile.GetPolicies()->GetPolicyID( + argP1->c_str(), pid), + reducible, arg, newArgs, argP1, argP2); } // does a target exist - if (*arg == "TARGET" && argP1 != newArgs.end()) + if (this->IsKeyword("TARGET", *arg) && argP1 != newArgs.end()) { this->HandlePredicate( - this->Makefile.FindTargetToUse(*argP1)?true:false, + this->Makefile.FindTargetToUse(argP1->GetValue())?true:false, reducible, arg, newArgs, argP1, argP2); } // is a variable defined - if (*arg == "DEFINED" && argP1 != newArgs.end()) + if (this->IsKeyword("DEFINED", *arg) && argP1 != newArgs.end()) { - size_t argP1len = argP1->size(); + size_t argP1len = argP1->GetValue().size(); bool bdef = false; - if(argP1len > 4 && argP1->substr(0, 4) == "ENV{" && - argP1->operator[](argP1len-1) == '}') + if(argP1len > 4 && argP1->GetValue().substr(0, 4) == "ENV{" && + argP1->GetValue().operator[](argP1len-1) == '}') { - std::string env = argP1->substr(4, argP1len-5); + std::string env = argP1->GetValue().substr(4, argP1len-5); bdef = cmSystemTools::GetEnv(env.c_str())?true:false; } else { - bdef = this->Makefile.IsDefinitionSet(*(argP1)); + bdef = this->Makefile.IsDefinitionSet(argP1->GetValue()); } this->HandlePredicate(bdef, reducible, arg, newArgs, argP1, argP2); } @@ -467,18 +543,18 @@ bool cmConditionEvaluator::HandleLevel2(cmArgumentList &newArgs, do { reducible = 0; - std::list::iterator arg = newArgs.begin(); - std::list::iterator argP1; - std::list::iterator argP2; + cmArgumentList::iterator arg = newArgs.begin(); + cmArgumentList::iterator argP1; + cmArgumentList::iterator argP2; while (arg != newArgs.end()) { argP1 = arg; this->IncrementArguments(newArgs,argP1,argP2); if (argP1 != newArgs.end() && argP2 != newArgs.end() && - *(argP1) == "MATCHES") + IsKeyword("MATCHES", *argP1)) { def = this->GetVariableOrString(*arg); - const char* rex = (argP2)->c_str(); + const char* rex = argP2->c_str(); this->Makefile.ClearMatches(); cmsys::RegularExpression regEntry; if ( !regEntry.compile(rex) ) @@ -492,11 +568,11 @@ bool cmConditionEvaluator::HandleLevel2(cmArgumentList &newArgs, if (regEntry.find(def)) { this->Makefile.StoreMatches(regEntry); - *arg = "1"; + *arg = cmExpandedCommandArgument("1", true); } else { - *arg = "0"; + *arg = cmExpandedCommandArgument("0", true); } newArgs.erase(argP2); newArgs.erase(argP1); @@ -505,9 +581,9 @@ bool cmConditionEvaluator::HandleLevel2(cmArgumentList &newArgs, reducible = 1; } - if (argP1 != newArgs.end() && *arg == "MATCHES") + if (argP1 != newArgs.end() && this->IsKeyword("MATCHES", *arg)) { - *arg = "0"; + *arg = cmExpandedCommandArgument("0", true); newArgs.erase(argP1); argP1 = arg; this->IncrementArguments(newArgs,argP1,argP2); @@ -515,8 +591,9 @@ bool cmConditionEvaluator::HandleLevel2(cmArgumentList &newArgs, } if (argP1 != newArgs.end() && argP2 != newArgs.end() && - (*(argP1) == "LESS" || *(argP1) == "GREATER" || - *(argP1) == "EQUAL")) + (this->IsKeyword("LESS", *argP1) || + this->IsKeyword("GREATER", *argP1) || + this->IsKeyword("EQUAL", *argP1))) { def = this->GetVariableOrString(*arg); def2 = this->GetVariableOrString(*argP2); @@ -540,14 +617,14 @@ bool cmConditionEvaluator::HandleLevel2(cmArgumentList &newArgs, { result = (lhs == rhs); } - HandleBinaryOp(result, + this->HandleBinaryOp(result, reducible, arg, newArgs, argP1, argP2); } if (argP1 != newArgs.end() && argP2 != newArgs.end() && - (*(argP1) == "STRLESS" || - *(argP1) == "STREQUAL" || - *(argP1) == "STRGREATER")) + (this->IsKeyword("STRLESS", *argP1) || + this->IsKeyword("STREQUAL", *argP1) || + this->IsKeyword("STRGREATER", *argP1))) { def = this->GetVariableOrString(*arg); def2 = this->GetVariableOrString(*argP2); @@ -570,8 +647,9 @@ bool cmConditionEvaluator::HandleLevel2(cmArgumentList &newArgs, } if (argP1 != newArgs.end() && argP2 != newArgs.end() && - (*(argP1) == "VERSION_LESS" || *(argP1) == "VERSION_GREATER" || - *(argP1) == "VERSION_EQUAL")) + (this->IsKeyword("VERSION_LESS", *argP1) || + this->IsKeyword("VERSION_GREATER", *argP1) || + this->IsKeyword("VERSION_EQUAL", *argP1))) { def = this->GetVariableOrString(*arg); def2 = this->GetVariableOrString(*argP2); @@ -591,11 +669,11 @@ bool cmConditionEvaluator::HandleLevel2(cmArgumentList &newArgs, // is file A newer than file B if (argP1 != newArgs.end() && argP2 != newArgs.end() && - *(argP1) == "IS_NEWER_THAN") + this->IsKeyword("IS_NEWER_THAN", *argP1)) { int fileIsNewer=0; - bool success=cmSystemTools::FileTimeCompare(arg->c_str(), - (argP2)->c_str(), + bool success=cmSystemTools::FileTimeCompare(arg->GetValue(), + (argP2)->GetValue(), &fileIsNewer); this->HandleBinaryOp( (success==false || fileIsNewer==1 || fileIsNewer==0), @@ -619,14 +697,14 @@ bool cmConditionEvaluator::HandleLevel3(cmArgumentList &newArgs, do { reducible = 0; - std::list::iterator arg = newArgs.begin(); - std::list::iterator argP1; - std::list::iterator argP2; + cmArgumentList::iterator arg = newArgs.begin(); + cmArgumentList::iterator argP1; + cmArgumentList::iterator argP2; while (arg != newArgs.end()) { argP1 = arg; IncrementArguments(newArgs,argP1,argP2); - if (argP1 != newArgs.end() && *arg == "NOT") + if (argP1 != newArgs.end() && IsKeyword("NOT", *arg)) { bool rhs = this->GetBooleanValueWithAutoDereference(*argP1, errorString, @@ -652,14 +730,14 @@ bool cmConditionEvaluator::HandleLevel4(cmArgumentList &newArgs, do { reducible = 0; - std::list::iterator arg = newArgs.begin(); - std::list::iterator argP1; - std::list::iterator argP2; + cmArgumentList::iterator arg = newArgs.begin(); + cmArgumentList::iterator argP1; + cmArgumentList::iterator argP2; while (arg != newArgs.end()) { argP1 = arg; IncrementArguments(newArgs,argP1,argP2); - if (argP1 != newArgs.end() && *(argP1) == "AND" && + if (argP1 != newArgs.end() && IsKeyword("AND", *argP1) && argP2 != newArgs.end()) { lhs = this->GetBooleanValueWithAutoDereference(*arg, @@ -672,7 +750,7 @@ bool cmConditionEvaluator::HandleLevel4(cmArgumentList &newArgs, reducible, arg, newArgs, argP1, argP2); } - if (argP1 != newArgs.end() && *(argP1) == "OR" && + if (argP1 != newArgs.end() && this->IsKeyword("OR", *argP1) && argP2 != newArgs.end()) { lhs = this->GetBooleanValueWithAutoDereference(*arg, diff --git a/Source/cmConditionEvaluator.h b/Source/cmConditionEvaluator.h index 9076988..01624f9 100644 --- a/Source/cmConditionEvaluator.h +++ b/Source/cmConditionEvaluator.h @@ -13,36 +13,41 @@ #define cmConditionEvaluator_h #include "cmCommand.h" +#include "cmExpandedCommandArgument.h" class cmConditionEvaluator { public: - typedef std::list cmArgumentList; + typedef std::list cmArgumentList; cmConditionEvaluator(cmMakefile& makefile); // this is a shared function for both If and Else to determine if the // arguments were valid, and if so, was the response true. If there is // an error, the errorString will be set. - bool IsTrue(const std::vector &args, + bool IsTrue(const std::vector &args, std::string &errorString, cmake::MessageType &status); private: + // Filter the given variable definition based on policy CMP0054. + const char* GetDefinitionIfUnquoted( + const cmExpandedCommandArgument& argument) const; + const char* GetVariableOrString( - const std::string& argument) const; + const cmExpandedCommandArgument& argument) const; bool IsKeyword(std::string const& keyword, - std::string& argument) const; + cmExpandedCommandArgument& argument) const; bool GetBooleanValue( - std::string& arg) const; + cmExpandedCommandArgument& arg) const; bool GetBooleanValueOld( - std::string const& arg, bool one) const; + cmExpandedCommandArgument const& arg, bool one) const; bool GetBooleanValueWithAutoDereference( - std::string &newArg, + cmExpandedCommandArgument &newArg, std::string &errorString, cmake::MessageType &status, bool oneArg = false) const; @@ -85,6 +90,7 @@ private: cmMakefile& Makefile; cmPolicies::PolicyStatus Policy12Status; + cmPolicies::PolicyStatus Policy54Status; }; #endif diff --git a/Source/cmExpandedCommandArgument.cxx b/Source/cmExpandedCommandArgument.cxx new file mode 100644 index 0000000..4477cf5 --- /dev/null +++ b/Source/cmExpandedCommandArgument.cxx @@ -0,0 +1,51 @@ +/*============================================================================ + CMake - Cross Platform Makefile Generator + Copyright 2014 Kitware, Inc., Insight Software Consortium + + Distributed under the OSI-approved BSD License (the "License"); + see accompanying file Copyright.txt for details. + + This software is distributed WITHOUT ANY WARRANTY; without even the + implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. + See the License for more information. +============================================================================*/ + +#include "cmExpandedCommandArgument.h" + +cmExpandedCommandArgument::cmExpandedCommandArgument(): + Quoted(false) +{ + +} + +cmExpandedCommandArgument::cmExpandedCommandArgument( + std::string const& value, bool quoted): + Value(value), Quoted(quoted) +{ + +} + +std::string const& cmExpandedCommandArgument::GetValue() const +{ + return this->Value; +} + +bool cmExpandedCommandArgument::WasQuoted() const +{ + return this->Quoted; +} + +bool cmExpandedCommandArgument::operator== (std::string const& value) const +{ + return this->Value == value; +} + +bool cmExpandedCommandArgument::empty() const +{ + return this->Value.empty(); +} + +const char* cmExpandedCommandArgument::c_str() const +{ + return this->Value.c_str(); +} diff --git a/Source/cmExpandedCommandArgument.h b/Source/cmExpandedCommandArgument.h new file mode 100644 index 0000000..f4e1517 --- /dev/null +++ b/Source/cmExpandedCommandArgument.h @@ -0,0 +1,45 @@ +/*============================================================================ + CMake - Cross Platform Makefile Generator + Copyright 2014 Kitware, Inc., Insight Software Consortium + + Distributed under the OSI-approved BSD License (the "License"); + see accompanying file Copyright.txt for details. + + This software is distributed WITHOUT ANY WARRANTY; without even the + implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. + See the License for more information. +============================================================================*/ +#ifndef cmExpandedCommandArgument_h +#define cmExpandedCommandArgument_h + +#include "cmStandardIncludes.h" + +/** \class cmExpandedCommandArgument + * \brief Represents an expanded command argument + * + * cmCommandArgument stores a string representing an expanded + * command argument and context information. + */ + +class cmExpandedCommandArgument +{ +public: + cmExpandedCommandArgument(); + cmExpandedCommandArgument(std::string const& value, bool quoted); + + std::string const& GetValue() const; + + bool WasQuoted() const; + + bool operator== (std::string const& value) const; + + bool empty() const; + + const char* c_str() const; + +private: + std::string Value; + bool Quoted; +}; + +#endif diff --git a/Source/cmIfCommand.cxx b/Source/cmIfCommand.cxx index 25ee25a..f728c15 100644 --- a/Source/cmIfCommand.cxx +++ b/Source/cmIfCommand.cxx @@ -20,15 +20,15 @@ static std::string cmIfCommandError( - cmMakefile* mf, std::vector const& args) + cmMakefile* mf, std::vector const& args) { cmLocalGenerator* lg = mf->GetLocalGenerator(); std::string err = "given arguments:\n "; - for(std::vector::const_iterator i = args.begin(); + for(std::vector::const_iterator i = args.begin(); i != args.end(); ++i) { err += " "; - err += lg->EscapeForCMake(*i); + err += lg->EscapeForCMake(i->GetValue()); } err += "\n"; return err; @@ -105,7 +105,7 @@ IsFunctionBlocked(const cmListFileFunction& lff, std::string errorString; - std::vector expandedArguments; + std::vector expandedArguments; mf.ExpandArguments(this->Functions[c].Arguments, expandedArguments); @@ -189,7 +189,7 @@ bool cmIfCommand { std::string errorString; - std::vector expandedArguments; + std::vector expandedArguments; this->Makefile->ExpandArguments(args, expandedArguments); cmake::MessageType status; diff --git a/Source/cmIfCommand.h b/Source/cmIfCommand.h index ea2a68f..689efce 100644 --- a/Source/cmIfCommand.h +++ b/Source/cmIfCommand.h @@ -70,6 +70,10 @@ public: */ virtual bool IsScriptable() const { return true; } + // Filter the given variable definition based on policy CMP0054. + static const char* GetDefinitionIfUnquoted( + const cmMakefile* mf, cmExpandedCommandArgument const& argument); + cmTypeMacro(cmIfCommand, cmCommand); }; diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 04b2d27..3017d15 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -3292,6 +3292,7 @@ void cmMakefile::PopFunctionBlockerBarrier(bool reportError) this->FunctionBlockerBarriers.pop_back(); } +//---------------------------------------------------------------------------- bool cmMakefile::ExpandArguments( std::vector const& inArgs, std::vector& outArgs) const @@ -3328,6 +3329,47 @@ bool cmMakefile::ExpandArguments( } //---------------------------------------------------------------------------- +bool cmMakefile::ExpandArguments( + std::vector const& inArgs, + std::vector& outArgs) const +{ + std::vector::const_iterator i; + std::string value; + outArgs.reserve(inArgs.size()); + for(i = inArgs.begin(); i != inArgs.end(); ++i) + { + // No expansion in a bracket argument. + if(i->Delim == cmListFileArgument::Bracket) + { + outArgs.push_back(cmExpandedCommandArgument(i->Value, true)); + continue; + } + // Expand the variables in the argument. + value = i->Value; + this->ExpandVariablesInString(value, false, false, false, + i->FilePath, i->Line, + false, false); + + // If the argument is quoted, it should be one argument. + // Otherwise, it may be a list of arguments. + if(i->Delim == cmListFileArgument::Quoted) + { + outArgs.push_back(cmExpandedCommandArgument(value, true)); + } + else + { + std::vector stringArgs; + cmSystemTools::ExpandListArgument(value, stringArgs); + for(size_t j = 0; j < stringArgs.size(); ++j) + { + outArgs.push_back(cmExpandedCommandArgument(stringArgs[j], false)); + } + } + } + return !cmSystemTools::GetFatalErrorOccured(); +} + +//---------------------------------------------------------------------------- void cmMakefile::AddFunctionBlocker(cmFunctionBlocker* fb) { if(!this->CallStack.empty()) @@ -4938,12 +4980,14 @@ void cmMakefile::PopPolicyBarrier(bool reportError) this->PolicyBarriers.pop_back(); } +//---------------------------------------------------------------------------- bool cmMakefile::SetPolicyVersion(const char *version) { return this->GetCMakeInstance()->GetPolicies()-> ApplyPolicyVersion(this,version); } +//---------------------------------------------------------------------------- cmPolicies *cmMakefile::GetPolicies() const { if (!this->GetCMakeInstance()) @@ -4954,6 +4998,23 @@ cmPolicies *cmMakefile::GetPolicies() const } //---------------------------------------------------------------------------- +bool cmMakefile::HasCMP0054AlreadyBeenReported( + cmListFileContext context) const +{ + cmCMP0054Id id(context); + + bool alreadyReported = + this->CMP0054ReportedIds.find(id) != this->CMP0054ReportedIds.end(); + + if(!alreadyReported) + { + this->CMP0054ReportedIds.insert(id); + } + + return alreadyReported; +} + +//---------------------------------------------------------------------------- void cmMakefile::RecordPolicies(cmPolicies::PolicyMap& pm) { /* Record the setting of every policy. */ diff --git a/Source/cmMakefile.h b/Source/cmMakefile.h index d728a62..164290a 100644 --- a/Source/cmMakefile.h +++ b/Source/cmMakefile.h @@ -21,6 +21,7 @@ #include "cmTarget.h" #include "cmNewLineStyle.h" #include "cmGeneratorTarget.h" +#include "cmExpandedCommandArgument.h" #include "cmake.h" #if defined(CMAKE_BUILD_WITH_CMAKE) @@ -375,7 +376,35 @@ public: /** * Get the Policies Instance */ - cmPolicies *GetPolicies() const; + cmPolicies *GetPolicies() const; + + struct cmCMP0054Id + { + cmCMP0054Id(cmListFileContext const& context): + Context(context) + { + + } + + bool operator< (cmCMP0054Id const& id) const + { + if(this->Context.FilePath != id.Context.FilePath) + return this->Context.FilePath < id.Context.FilePath; + + return this->Context.Line < id.Context.Line; + } + + cmListFileContext Context; + }; + + mutable std::set CMP0054ReportedIds; + + /** + * Determine if the given context, name pair has already been reported + * in context of CMP0054. + */ + bool HasCMP0054AlreadyBeenReported( + cmListFileContext context) const; /** * Add an auxiliary directory to the build. @@ -770,6 +799,10 @@ public: */ bool ExpandArguments(std::vector const& inArgs, std::vector& outArgs) const; + + bool ExpandArguments(std::vector const& inArgs, + std::vector& outArgs) const; + /** * Get the instance */ diff --git a/Source/cmPolicies.cxx b/Source/cmPolicies.cxx index 693945d..a420f59 100644 --- a/Source/cmPolicies.cxx +++ b/Source/cmPolicies.cxx @@ -359,6 +359,11 @@ cmPolicies::cmPolicies() CMP0053, "CMP0053", "Simplify variable reference and escape sequence evaluation.", 3,1,0, cmPolicies::WARN); + + this->DefinePolicy( + CMP0054, "CMP0054", + "Only interpret if() arguments as variables or keywords when unquoted.", + 3,1,0, cmPolicies::WARN); } cmPolicies::~cmPolicies() diff --git a/Source/cmPolicies.h b/Source/cmPolicies.h index 5d69d14..7c73da8 100644 --- a/Source/cmPolicies.h +++ b/Source/cmPolicies.h @@ -109,6 +109,8 @@ public: /// INTERFACE_INCLUDE_DIRECTORIES CMP0053, ///< Simplify variable reference and escape sequence evaluation + CMP0054, ///< Only interpret if() arguments as variables + /// or keywords when unquoted. /** \brief Always the last entry. * diff --git a/Source/cmWhileCommand.cxx b/Source/cmWhileCommand.cxx index 5565b8a..851c4cb 100644 --- a/Source/cmWhileCommand.cxx +++ b/Source/cmWhileCommand.cxx @@ -34,7 +34,7 @@ IsFunctionBlocked(const cmListFileFunction& lff, cmMakefile &mf, std::string errorString; - std::vector expandedArguments; + std::vector expandedArguments; mf.ExpandArguments(this->Args, expandedArguments); cmake::MessageType messageType; diff --git a/Tests/RunCMake/CMP0054/CMP0054-NEW-stderr.txt b/Tests/RunCMake/CMP0054/CMP0054-NEW-stderr.txt new file mode 100644 index 0000000..f5a8fbe --- /dev/null +++ b/Tests/RunCMake/CMP0054/CMP0054-NEW-stderr.txt @@ -0,0 +1 @@ +$^ diff --git a/Tests/RunCMake/CMP0054/CMP0054-NEW.cmake b/Tests/RunCMake/CMP0054/CMP0054-NEW.cmake new file mode 100644 index 0000000..23a9124 --- /dev/null +++ b/Tests/RunCMake/CMP0054/CMP0054-NEW.cmake @@ -0,0 +1,47 @@ +cmake_policy(SET CMP0054 NEW) + +set(FOO "BAR") +set(BAZ "ZZZ") +set(MYTRUE ON) +set(MYNUMBER 3) +set(MYVERSION 3.0) + +function(assert_unquoted PREFIX FIRST) + string(REPLACE ";" " " ARGN_SP "${ARGN}") + if(${PREFIX} ${FIRST} ${ARGN}) + message(FATAL_ERROR "Assertion failed [${PREFIX} ${FIRST} ${ARGN_SP}]") + endif() +endfunction() + +function(assert_quoted PREFIX FIRST) + string(REPLACE ";" " " ARGN_SP "${ARGN}") + if(${PREFIX} "${FIRST}" ${ARGN}) + message(FATAL_ERROR "Assertion failed [${PREFIX} \"${FIRST}\" ${ARGN_SP}]") + endif() +endfunction() + +function(assert FIRST) + assert_unquoted(NOT ${FIRST} ${ARGN}) + assert_quoted("" ${FIRST} ${ARGN}) +endfunction() + +assert(MYTRUE) + +assert(FOO MATCHES "^BAR$") + +assert(MYNUMBER LESS 4) +assert(MYNUMBER GREATER 2) +assert(MYNUMBER EQUAL 3) + +assert(FOO STRLESS CCC) +assert(BAZ STRGREATER CCC) +assert(FOO STREQUAL BAR) + +assert_unquoted(NOT MYVERSION VERSION_LESS 3.1) +assert_unquoted("" MYVERSION VERSION_LESS 2.9) + +assert_quoted(NOT MYVERSION VERSION_LESS 2.9) +assert_quoted(NOT MYVERSION VERSION_LESS 3.1) + +assert(MYVERSION VERSION_GREATER 2.9) +assert(MYVERSION VERSION_EQUAL 3.0) diff --git a/Tests/RunCMake/CMP0054/CMP0054-OLD-stderr.txt b/Tests/RunCMake/CMP0054/CMP0054-OLD-stderr.txt new file mode 100644 index 0000000..f5a8fbe --- /dev/null +++ b/Tests/RunCMake/CMP0054/CMP0054-OLD-stderr.txt @@ -0,0 +1 @@ +$^ diff --git a/Tests/RunCMake/CMP0054/CMP0054-OLD.cmake b/Tests/RunCMake/CMP0054/CMP0054-OLD.cmake new file mode 100644 index 0000000..0c4cede --- /dev/null +++ b/Tests/RunCMake/CMP0054/CMP0054-OLD.cmake @@ -0,0 +1,47 @@ +cmake_policy(SET CMP0054 OLD) + +set(FOO "BAR") +set(BAZ "ZZZ") +set(MYTRUE ON) +set(MYNUMBER 3) +set(MYVERSION 3.0) + +function(assert_unquoted PREFIX FIRST) + string(REPLACE ";" " " ARGN_SP "${ARGN}") + if(${PREFIX} ${FIRST} ${ARGN}) + message(FATAL_ERROR "Assertion failed [${PREFIX} ${FIRST} ${ARGN_SP}]") + endif() +endfunction() + +function(assert_quoted PREFIX FIRST) + string(REPLACE ";" " " ARGN_SP "${ARGN}") + if(${PREFIX} "${FIRST}" ${ARGN}) + message(FATAL_ERROR "Assertion failed [${PREFIX} \"${FIRST}\" ${ARGN_SP}]") + endif() +endfunction() + +function(assert FIRST) + assert_unquoted(NOT ${FIRST} ${ARGN}) + assert_quoted(NOT ${FIRST} ${ARGN}) +endfunction() + +assert(MYTRUE) + +assert(FOO MATCHES "^BAR$") + +assert(MYNUMBER LESS 4) +assert(MYNUMBER GREATER 2) +assert(MYNUMBER EQUAL 3) + +assert(FOO STRLESS CCC) +assert(BAZ STRGREATER CCC) +assert(FOO STREQUAL BAR) + +assert_unquoted(NOT MYVERSION VERSION_LESS 3.1) +assert_unquoted("" MYVERSION VERSION_LESS 2.9) + +assert_quoted("" MYVERSION VERSION_LESS 2.9) +assert_quoted(NOT MYVERSION VERSION_LESS 3.1) + +assert(MYVERSION VERSION_GREATER 2.9) +assert(MYVERSION VERSION_EQUAL 3.0) diff --git a/Tests/RunCMake/CMP0054/CMP0054-WARN-stderr.txt b/Tests/RunCMake/CMP0054/CMP0054-WARN-stderr.txt new file mode 100644 index 0000000..3d875ae --- /dev/null +++ b/Tests/RunCMake/CMP0054/CMP0054-WARN-stderr.txt @@ -0,0 +1,11 @@ +CMake Warning \(dev\) at CMP0054-WARN.cmake:3 \(if\): + Policy CMP0054 is not set: Only interpret if\(\) arguments as variables or + keywords when unquoted. Run "cmake --help-policy CMP0054" for policy + details. Use the cmake_policy command to set the policy and suppress this + warning. + + Quoted variables like "FOO" will no longer be dereferenced when the policy + is set to NEW. Since the policy is not set the OLD behavior will be used. +Call Stack \(most recent call first\): + CMakeLists.txt:3 \(include\) +This warning is for project developers. Use -Wno-dev to suppress it. diff --git a/Tests/RunCMake/CMP0054/CMP0054-WARN.cmake b/Tests/RunCMake/CMP0054/CMP0054-WARN.cmake new file mode 100644 index 0000000..37855fc --- /dev/null +++ b/Tests/RunCMake/CMP0054/CMP0054-WARN.cmake @@ -0,0 +1,5 @@ +set(FOO "BAR") + +if(NOT "FOO" STREQUAL "BAR") + message(FATAL_ERROR "The given literals should match") +endif() diff --git a/Tests/RunCMake/CMP0054/CMP0054-duplicate-warnings-stderr.txt b/Tests/RunCMake/CMP0054/CMP0054-duplicate-warnings-stderr.txt new file mode 100644 index 0000000..485db52 --- /dev/null +++ b/Tests/RunCMake/CMP0054/CMP0054-duplicate-warnings-stderr.txt @@ -0,0 +1,24 @@ +CMake Warning \(dev\) at CMP0054-duplicate-warnings.cmake:4 \(if\): + Policy CMP0054 is not set: Only interpret if\(\) arguments as variables or + keywords when unquoted. Run "cmake --help-policy CMP0054" for policy + details. Use the cmake_policy command to set the policy and suppress this + warning. + + Quoted variables like "FOO" will no longer be dereferenced when the policy + is set to NEW. Since the policy is not set the OLD behavior will be used. +Call Stack \(most recent call first\): + CMP0054-duplicate-warnings.cmake:8 \(generate_warning\) + CMakeLists.txt:3 \(include\) +This warning is for project developers. Use -Wno-dev to suppress it. + +CMake Warning \(dev\) at CMP0054-duplicate-warnings.cmake:11 \(if\): + Policy CMP0054 is not set: Only interpret if\(\) arguments as variables or + keywords when unquoted. Run "cmake --help-policy CMP0054" for policy + details. Use the cmake_policy command to set the policy and suppress this + warning. + + Quoted variables like "FOO" will no longer be dereferenced when the policy + is set to NEW. Since the policy is not set the OLD behavior will be used. +Call Stack \(most recent call first\): + CMakeLists.txt:3 \(include\) +This warning is for project developers. Use -Wno-dev to suppress it. diff --git a/Tests/RunCMake/CMP0054/CMP0054-duplicate-warnings.cmake b/Tests/RunCMake/CMP0054/CMP0054-duplicate-warnings.cmake new file mode 100644 index 0000000..04fbe14 --- /dev/null +++ b/Tests/RunCMake/CMP0054/CMP0054-duplicate-warnings.cmake @@ -0,0 +1,12 @@ +set(FOO "BAR") + +function(generate_warning) + if("FOO" STREQUAL "BAR") + endif() +endfunction() + +generate_warning() +generate_warning() + +if("FOO" STREQUAL "BAR") +endif() diff --git a/Tests/RunCMake/CMP0054/CMP0054-keywords-NEW-result.txt b/Tests/RunCMake/CMP0054/CMP0054-keywords-NEW-result.txt new file mode 100644 index 0000000..d00491f --- /dev/null +++ b/Tests/RunCMake/CMP0054/CMP0054-keywords-NEW-result.txt @@ -0,0 +1 @@ +1 diff --git a/Tests/RunCMake/CMP0054/CMP0054-keywords-NEW-stderr.txt b/Tests/RunCMake/CMP0054/CMP0054-keywords-NEW-stderr.txt new file mode 100644 index 0000000..f444684 --- /dev/null +++ b/Tests/RunCMake/CMP0054/CMP0054-keywords-NEW-stderr.txt @@ -0,0 +1,8 @@ +CMake Error at CMP0054-keywords-NEW.cmake:23 \(if\): + if given arguments: + + "NOT" "1" + + Unknown arguments specified +Call Stack \(most recent call first\): + CMakeLists.txt:3 \(include\) diff --git a/Tests/RunCMake/CMP0054/CMP0054-keywords-NEW.cmake b/Tests/RunCMake/CMP0054/CMP0054-keywords-NEW.cmake new file mode 100644 index 0000000..b957658 --- /dev/null +++ b/Tests/RunCMake/CMP0054/CMP0054-keywords-NEW.cmake @@ -0,0 +1,25 @@ +cmake_policy(SET CMP0054 NEW) + +function(assert KEYWORD) + if("${KEYWORD}" STREQUAL "${KEYWORD}") + else() + message(FATAL_ERROR + "Assertion failed [\"${KEYWORD}\" STREQUAL \"${KEYWORD}\"]") + endif() +endfunction() + +assert("NOT") +assert("COMMAND") +assert("POLICY") +assert("TARGET") +assert("EXISTS") +assert("IS_DIRECTORY") +assert("IS_SYMLINK") +assert("IS_ABSOLUTE") +assert("DEFINED") +assert("(") +assert(")") + +if("NOT" 1) + message(FATAL_ERROR "[\"NOT\" 1] evaluated true") +endif() diff --git a/Tests/RunCMake/CMP0054/CMP0054-keywords-OLD-stderr.txt b/Tests/RunCMake/CMP0054/CMP0054-keywords-OLD-stderr.txt new file mode 100644 index 0000000..f5a8fbe --- /dev/null +++ b/Tests/RunCMake/CMP0054/CMP0054-keywords-OLD-stderr.txt @@ -0,0 +1 @@ +$^ diff --git a/Tests/RunCMake/CMP0054/CMP0054-keywords-OLD.cmake b/Tests/RunCMake/CMP0054/CMP0054-keywords-OLD.cmake new file mode 100644 index 0000000..a2a7f51 --- /dev/null +++ b/Tests/RunCMake/CMP0054/CMP0054-keywords-OLD.cmake @@ -0,0 +1,9 @@ +cmake_policy(SET CMP0054 OLD) + +if(NOT 1) + message(FATAL_ERROR "[NOT 1] evaluated true") +endif() + +if("NOT" 1) + message(FATAL_ERROR "[\"NOT\" 1] evaluated true") +endif() diff --git a/Tests/RunCMake/CMP0054/CMP0054-keywords-WARN-stderr.txt b/Tests/RunCMake/CMP0054/CMP0054-keywords-WARN-stderr.txt new file mode 100644 index 0000000..b1ebd49 --- /dev/null +++ b/Tests/RunCMake/CMP0054/CMP0054-keywords-WARN-stderr.txt @@ -0,0 +1,12 @@ +CMake Warning \(dev\) at CMP0054-keywords-WARN.cmake:1 \(if\): + Policy CMP0054 is not set: Only interpret if\(\) arguments as variables or + keywords when unquoted. Run "cmake --help-policy CMP0054" for policy + details. Use the cmake_policy command to set the policy and suppress this + warning. + + Quoted keywords like "NOT" will no longer be interpreted as keywords when + the policy is set to NEW. Since the policy is not set the OLD behavior + will be used. +Call Stack \(most recent call first\): + CMakeLists.txt:3 \(include\) +This warning is for project developers. Use -Wno-dev to suppress it. diff --git a/Tests/RunCMake/CMP0054/CMP0054-keywords-WARN.cmake b/Tests/RunCMake/CMP0054/CMP0054-keywords-WARN.cmake new file mode 100644 index 0000000..ee0a623 --- /dev/null +++ b/Tests/RunCMake/CMP0054/CMP0054-keywords-WARN.cmake @@ -0,0 +1,3 @@ +if("NOT" 1) + message(FATAL_ERROR "[\"NOT\" 1] evaluated true") +endif() diff --git a/Tests/RunCMake/CMP0054/CMP0054-policy-command-scope-stderr.txt b/Tests/RunCMake/CMP0054/CMP0054-policy-command-scope-stderr.txt new file mode 100644 index 0000000..f5a8fbe --- /dev/null +++ b/Tests/RunCMake/CMP0054/CMP0054-policy-command-scope-stderr.txt @@ -0,0 +1 @@ +$^ diff --git a/Tests/RunCMake/CMP0054/CMP0054-policy-command-scope.cmake b/Tests/RunCMake/CMP0054/CMP0054-policy-command-scope.cmake new file mode 100644 index 0000000..b6b243c --- /dev/null +++ b/Tests/RunCMake/CMP0054/CMP0054-policy-command-scope.cmake @@ -0,0 +1,53 @@ +set(FOO BAR) + +cmake_policy(SET CMP0054 NEW) + +function(function_defined_new_called_old) + if(NOT FOO STREQUAL BAR) + message(FATAL_ERROR "The variable should match the string") + endif() + + if("FOO" STREQUAL BAR) + message(FATAL_ERROR "The strings should not match") + endif() +endfunction() + +macro(macro_defined_new_called_old) + if(NOT FOO STREQUAL BAR) + message(FATAL_ERROR "The variable should match the string") + endif() + + if("FOO" STREQUAL BAR) + message(FATAL_ERROR "The strings should not match") + endif() +endmacro() + +cmake_policy(SET CMP0054 OLD) + +function_defined_new_called_old() +macro_defined_new_called_old() + +function(function_defined_old_called_new) + if(NOT FOO STREQUAL BAR) + message(FATAL_ERROR "The variable should match the string") + endif() + + if(NOT "FOO" STREQUAL BAR) + message(FATAL_ERROR "The quoted variable should match the string") + endif() +endfunction() + +macro(macro_defined_old_called_new) + if(NOT FOO STREQUAL BAR) + message(FATAL_ERROR "The variable should match the string") + endif() + + if(NOT "FOO" STREQUAL BAR) + message(FATAL_ERROR "The quoted variable should match the string") + endif() +endmacro() + +cmake_policy(SET CMP0054 NEW) + +function_defined_old_called_new() +macro_defined_old_called_new() diff --git a/Tests/RunCMake/CMP0054/CMP0054-policy-foreach-scope-stderr.txt b/Tests/RunCMake/CMP0054/CMP0054-policy-foreach-scope-stderr.txt new file mode 100644 index 0000000..f5a8fbe --- /dev/null +++ b/Tests/RunCMake/CMP0054/CMP0054-policy-foreach-scope-stderr.txt @@ -0,0 +1 @@ +$^ diff --git a/Tests/RunCMake/CMP0054/CMP0054-policy-foreach-scope.cmake b/Tests/RunCMake/CMP0054/CMP0054-policy-foreach-scope.cmake new file mode 100644 index 0000000..87c968a --- /dev/null +++ b/Tests/RunCMake/CMP0054/CMP0054-policy-foreach-scope.cmake @@ -0,0 +1,49 @@ +set(FOO BAR) + +cmake_policy(SET CMP0054 NEW) + +foreach(loop_var arg1 arg2) + if(NOT FOO STREQUAL BAR) + message(FATAL_ERROR "The variable should match the string") + endif() + + if("FOO" STREQUAL BAR) + message(FATAL_ERROR "The strings should not match") + endif() + + cmake_policy(SET CMP0054 OLD) + + if(NOT FOO STREQUAL BAR) + message(FATAL_ERROR "The variable should match the string") + endif() + + if(NOT "FOO" STREQUAL BAR) + message(FATAL_ERROR "The quoted variable should match the string") + endif() + + cmake_policy(SET CMP0054 NEW) +endforeach() + +cmake_policy(SET CMP0054 OLD) + +foreach(loop_var arg1 arg2) + if(NOT FOO STREQUAL BAR) + message(FATAL_ERROR "The variable should match the string") + endif() + + if(NOT "FOO" STREQUAL BAR) + message(FATAL_ERROR "The quoted variable should match the string") + endif() + + cmake_policy(SET CMP0054 NEW) + + if(NOT FOO STREQUAL BAR) + message(FATAL_ERROR "The variable should match the string") + endif() + + if("FOO" STREQUAL BAR) + message(FATAL_ERROR "The strings should not match") + endif() + + cmake_policy(SET CMP0054 OLD) +endforeach() diff --git a/Tests/RunCMake/CMP0054/CMP0054-policy-nested-if-stderr.txt b/Tests/RunCMake/CMP0054/CMP0054-policy-nested-if-stderr.txt new file mode 100644 index 0000000..f5a8fbe --- /dev/null +++ b/Tests/RunCMake/CMP0054/CMP0054-policy-nested-if-stderr.txt @@ -0,0 +1 @@ +$^ diff --git a/Tests/RunCMake/CMP0054/CMP0054-policy-nested-if.cmake b/Tests/RunCMake/CMP0054/CMP0054-policy-nested-if.cmake new file mode 100644 index 0000000..dd7434d --- /dev/null +++ b/Tests/RunCMake/CMP0054/CMP0054-policy-nested-if.cmake @@ -0,0 +1,41 @@ +set(FOO BAR) + +cmake_policy(SET CMP0054 NEW) + +if("FOO" STREQUAL BAR) + message(FATAL_ERROR "The strings should not match") + + if("FOO" STREQUAL BAR) + message(FATAL_ERROR "The strings should not match") + endif() + + cmake_policy(SET CMP0054 OLD) + + if(NOT "FOO" STREQUAL BAR) + message(FATAL_ERROR "The quoted variable should match the string") + endif() +endif() + +if("FOO" STREQUAL BAR) + message(FATAL_ERROR "The strings should not match") +endif() + +cmake_policy(SET CMP0054 OLD) + +if(NOT "FOO" STREQUAL BAR) + message(FATAL_ERROR "The quoted variable should match the string") + + if(NOT "FOO" STREQUAL BAR) + message(FATAL_ERROR "The quoted variable should match the string") + endif() + + cmake_policy(SET CMP0054 NEW) + + if("FOO" STREQUAL BAR) + message(FATAL_ERROR "The strings should not match") + endif() +endif() + +if(NOT "FOO" STREQUAL BAR) + message(FATAL_ERROR "The quoted variable should match the string") +endif() diff --git a/Tests/RunCMake/CMP0054/CMP0054-policy-while-scope-stderr.txt b/Tests/RunCMake/CMP0054/CMP0054-policy-while-scope-stderr.txt new file mode 100644 index 0000000..f5a8fbe --- /dev/null +++ b/Tests/RunCMake/CMP0054/CMP0054-policy-while-scope-stderr.txt @@ -0,0 +1 @@ +$^ diff --git a/Tests/RunCMake/CMP0054/CMP0054-policy-while-scope.cmake b/Tests/RunCMake/CMP0054/CMP0054-policy-while-scope.cmake new file mode 100644 index 0000000..2b22778 --- /dev/null +++ b/Tests/RunCMake/CMP0054/CMP0054-policy-while-scope.cmake @@ -0,0 +1,65 @@ +set(FOO BAR) + +set(LOOP_VAR "") + +cmake_policy(SET CMP0054 NEW) + +while(NOT LOOP_VAR STREQUAL "xx") + if(NOT FOO STREQUAL BAR) + message(FATAL_ERROR "The variable should match the string") + endif() + + if("FOO" STREQUAL BAR) + message(FATAL_ERROR "The strings should not match") + endif() + + cmake_policy(SET CMP0054 OLD) + + if(NOT FOO STREQUAL BAR) + message(FATAL_ERROR "The variable should match the string") + endif() + + if(NOT "FOO" STREQUAL BAR) + message(FATAL_ERROR "The quoted variable should match the string") + endif() + + cmake_policy(SET CMP0054 NEW) + + set(LOOP_VAR "${LOOP_VAR}x") +endwhile() + +while("FOO" STREQUAL BAR) + message(FATAL_ERROR "The strings should not match") +endwhile() + +set(LOOP_VAR "") + +cmake_policy(SET CMP0054 OLD) + +while(NOT LOOP_VAR STREQUAL "xx") + if(NOT FOO STREQUAL BAR) + message(FATAL_ERROR "The variable should match the string") + endif() + + if(NOT "FOO" STREQUAL BAR) + message(FATAL_ERROR "The quoted variable should match the string") + endif() + + cmake_policy(SET CMP0054 NEW) + + if(NOT FOO STREQUAL BAR) + message(FATAL_ERROR "The variable should match the string") + endif() + + if("FOO" STREQUAL BAR) + message(FATAL_ERROR "The strings should not match") + endif() + + cmake_policy(SET CMP0054 OLD) + + set(LOOP_VAR "${LOOP_VAR}x") +endwhile() + +if(NOT "FOO" STREQUAL BAR) + message(FATAL_ERROR "The quoted variable should match the string") +endif() diff --git a/Tests/RunCMake/CMP0054/CMakeLists.txt b/Tests/RunCMake/CMP0054/CMakeLists.txt new file mode 100644 index 0000000..2897109 --- /dev/null +++ b/Tests/RunCMake/CMP0054/CMakeLists.txt @@ -0,0 +1,3 @@ +cmake_minimum_required(VERSION 3.0) +project(${RunCMake_TEST} NONE) +include(${RunCMake_TEST}.cmake) diff --git a/Tests/RunCMake/CMP0054/RunCMakeTest.cmake b/Tests/RunCMake/CMP0054/RunCMakeTest.cmake new file mode 100644 index 0000000..2f2fb76 --- /dev/null +++ b/Tests/RunCMake/CMP0054/RunCMakeTest.cmake @@ -0,0 +1,13 @@ +include(RunCMake) + +run_cmake(CMP0054-OLD) +run_cmake(CMP0054-NEW) +run_cmake(CMP0054-WARN) +run_cmake(CMP0054-keywords-NEW) +run_cmake(CMP0054-keywords-OLD) +run_cmake(CMP0054-keywords-WARN) +run_cmake(CMP0054-duplicate-warnings) +run_cmake(CMP0054-policy-command-scope) +run_cmake(CMP0054-policy-foreach-scope) +run_cmake(CMP0054-policy-while-scope) +run_cmake(CMP0054-policy-nested-if) diff --git a/Tests/RunCMake/CMakeLists.txt b/Tests/RunCMake/CMakeLists.txt index 3cd9947..8f09c5f 100644 --- a/Tests/RunCMake/CMakeLists.txt +++ b/Tests/RunCMake/CMakeLists.txt @@ -47,6 +47,7 @@ add_RunCMake_test(CMP0049) add_RunCMake_test(CMP0050) add_RunCMake_test(CMP0051) add_RunCMake_test(CMP0053) +add_RunCMake_test(CMP0054) add_RunCMake_test(CTest) if(UNIX AND "${CMAKE_GENERATOR}" MATCHES "Unix Makefiles") add_RunCMake_test(CompilerChange) -- cgit v0.12 From 0b12815dc2feeb328a86b77328d13a328368d38b Mon Sep 17 00:00:00 2001 From: Brad King Date: Mon, 8 Sep 2014 09:58:33 -0400 Subject: Modules/Test*.cmake: Use if(DEFINED) to simplify conditions Replace old hacks of the form 'if("${VAR}" MATCHES "^${VAR}$")' with the much simpler 'if(NOT DEFINED ${VAR})'. --- Modules/TestBigEndian.cmake | 2 +- Modules/TestForANSIForScope.cmake | 2 +- Modules/TestForSSTREAM.cmake | 2 +- Modules/TestForSTDNamespace.cmake | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Modules/TestBigEndian.cmake b/Modules/TestBigEndian.cmake index 6f32b08..fcb41ab 100644 --- a/Modules/TestBigEndian.cmake +++ b/Modules/TestBigEndian.cmake @@ -25,7 +25,7 @@ # License text for the above reference.) macro(TEST_BIG_ENDIAN VARIABLE) - if("HAVE_${VARIABLE}" MATCHES "^HAVE_${VARIABLE}$") + if(NOT DEFINED HAVE_${VARIABLE}) message(STATUS "Check if the system is big endian") message(STATUS "Searching 16 bit integer") diff --git a/Modules/TestForANSIForScope.cmake b/Modules/TestForANSIForScope.cmake index de4b1f1..78fff9f 100644 --- a/Modules/TestForANSIForScope.cmake +++ b/Modules/TestForANSIForScope.cmake @@ -24,7 +24,7 @@ # (To distribute this file outside of CMake, substitute the full # License text for the above reference.) -if("CMAKE_ANSI_FOR_SCOPE" MATCHES "^CMAKE_ANSI_FOR_SCOPE$") +if(NOT DEFINED CMAKE_ANSI_FOR_SCOPE) message(STATUS "Check for ANSI scope") try_compile(CMAKE_ANSI_FOR_SCOPE ${CMAKE_BINARY_DIR} ${CMAKE_ROOT}/Modules/TestForAnsiForScope.cxx diff --git a/Modules/TestForSSTREAM.cmake b/Modules/TestForSSTREAM.cmake index 8977583..fe18ea2 100644 --- a/Modules/TestForSSTREAM.cmake +++ b/Modules/TestForSSTREAM.cmake @@ -23,7 +23,7 @@ # (To distribute this file outside of CMake, substitute the full # License text for the above reference.) -if("CMAKE_HAS_ANSI_STRING_STREAM" MATCHES "^CMAKE_HAS_ANSI_STRING_STREAM$") +if(NOT DEFINED CMAKE_HAS_ANSI_STRING_STREAM) message(STATUS "Check for sstream") try_compile(CMAKE_HAS_ANSI_STRING_STREAM ${CMAKE_BINARY_DIR} ${CMAKE_ROOT}/Modules/TestForSSTREAM.cxx diff --git a/Modules/TestForSTDNamespace.cmake b/Modules/TestForSTDNamespace.cmake index e43b75d..0d90774 100644 --- a/Modules/TestForSTDNamespace.cmake +++ b/Modules/TestForSTDNamespace.cmake @@ -23,7 +23,7 @@ # (To distribute this file outside of CMake, substitute the full # License text for the above reference.) -if("CMAKE_STD_NAMESPACE" MATCHES "^CMAKE_STD_NAMESPACE$") +if(NOT DEFINED CMAKE_STD_NAMESPACE) message(STATUS "Check for STD namespace") try_compile(CMAKE_STD_NAMESPACE ${CMAKE_BINARY_DIR} ${CMAKE_ROOT}/Modules/TestForSTDNamespace.cxx -- cgit v0.12 From 2d97178b30211b1984a17c82edb313fcddff7bc6 Mon Sep 17 00:00:00 2001 From: Brad King Date: Mon, 8 Sep 2014 09:59:47 -0400 Subject: FindGTK2: Avoid depending on if() to dereference a quoted variable Explicitly dereference GTK2_${_var}CONFIG_INCLUDE_DIR and GTK2_${_var}_INCLUDE_DIR when comparing their values. --- Modules/FindGTK2.cmake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/FindGTK2.cmake b/Modules/FindGTK2.cmake index 67dc0eb..72bb8eb 100644 --- a/Modules/FindGTK2.cmake +++ b/Modules/FindGTK2.cmake @@ -527,7 +527,7 @@ function(_GTK2_ADD_TARGET _var) set_property(TARGET GTK2::${_basename} APPEND PROPERTY INTERFACE_INCLUDE_DIRECTORIES "${GTK2_${_var}_INCLUDE_DIR}") endif() - if(GTK2_${_var}CONFIG_INCLUDE_DIR AND NOT "${GTK2_${_var}CONFIG_INCLUDE_DIR}" STREQUAL "GTK2_${_var}_INCLUDE_DIR") + if(GTK2_${_var}CONFIG_INCLUDE_DIR AND NOT "x${GTK2_${_var}CONFIG_INCLUDE_DIR}" STREQUAL "x${GTK2_${_var}_INCLUDE_DIR}") set_property(TARGET GTK2::${_basename} APPEND PROPERTY INTERFACE_INCLUDE_DIRECTORIES "${GTK2_${_var}CONFIG_INCLUDE_DIR}") endif() -- cgit v0.12 From cede5cbd53175be721ee2786cf0e482be3134fcf Mon Sep 17 00:00:00 2001 From: Brad King Date: Mon, 8 Sep 2014 10:01:47 -0400 Subject: libarchive: Avoid depending on if() to dereference a quoted variable --- Utilities/cmlibarchive/CMakeLists.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Utilities/cmlibarchive/CMakeLists.txt b/Utilities/cmlibarchive/CMakeLists.txt index 87a9c3f..f1459d4 100644 --- a/Utilities/cmlibarchive/CMakeLists.txt +++ b/Utilities/cmlibarchive/CMakeLists.txt @@ -840,14 +840,14 @@ ENDIF() # Check functions # CMAKE_PUSH_CHECK_STATE() # Save the state of the variables -IF ("CMAKE_C_COMPILER_ID" MATCHES "^GNU$") +IF (CMAKE_C_COMPILER_ID STREQUAL "GNU") # # During checking functions, we should use -fno-builtin to avoid the # failure of function detection which failure is an error "conflicting # types for built-in function" caused by using -Werror option. # SET(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -fno-builtin") -ENDIF ("CMAKE_C_COMPILER_ID" MATCHES "^GNU$") +ENDIF () CHECK_SYMBOL_EXISTS(_CrtSetReportMode "crtdbg.h" HAVE__CrtSetReportMode) CHECK_FUNCTION_EXISTS_GLIBC(chflags HAVE_CHFLAGS) CHECK_FUNCTION_EXISTS_GLIBC(chown HAVE_CHOWN) -- cgit v0.12 From 425acc522ff27014ffcc1d1f902f3702dd6c7aa7 Mon Sep 17 00:00:00 2001 From: Brad King Date: Mon, 8 Sep 2014 10:02:55 -0400 Subject: cmcurl: Use if(DEFINED) to simplify conditions Replace old hacks of the form 'if("${VAR}" MATCHES "^${VAR}$")' with the much simpler 'if(NOT DEFINED ${VAR})'. --- Utilities/cmcurl/CMake/CurlCheckCSourceCompiles.cmake | 4 ++-- Utilities/cmcurl/CMakeLists.txt | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Utilities/cmcurl/CMake/CurlCheckCSourceCompiles.cmake b/Utilities/cmcurl/CMake/CurlCheckCSourceCompiles.cmake index cfcf47b..2f427a2 100644 --- a/Utilities/cmcurl/CMake/CurlCheckCSourceCompiles.cmake +++ b/Utilities/cmcurl/CMake/CurlCheckCSourceCompiles.cmake @@ -13,7 +13,7 @@ # CMAKE_REQUIRED_LIBRARIES = list of libraries to link MACRO(CURL_CHECK_C_SOURCE_COMPILES SOURCE VAR) - IF("${VAR}" MATCHES "^${VAR}$" OR "${VAR}" MATCHES "UNKNOWN") + IF(NOT DEFINED ${VAR} OR ${VAR} MATCHES "UNKNOWN") SET(message "${VAR}") # If the number of arguments is greater than 2 (SOURCE VAR) IF(${ARGC} GREATER 2) @@ -70,5 +70,5 @@ MACRO(CURL_CHECK_C_SOURCE_COMPILES SOURCE VAR) "${OUTPUT}\n" "Source file was:\n${src}\n") ENDIF(${VAR}) - ENDIF("${VAR}" MATCHES "^${VAR}$" OR "${VAR}" MATCHES "UNKNOWN") + ENDIF() ENDMACRO(CURL_CHECK_C_SOURCE_COMPILES) diff --git a/Utilities/cmcurl/CMakeLists.txt b/Utilities/cmcurl/CMakeLists.txt index 1b918c9..03f10a0 100644 --- a/Utilities/cmcurl/CMakeLists.txt +++ b/Utilities/cmcurl/CMakeLists.txt @@ -505,7 +505,7 @@ ENDIF(NOT HAVE_SIGSETJMP) # For other curl specific tests, use this macro. MACRO(CURL_INTERNAL_TEST CURL_TEST) - IF("${CURL_TEST}" MATCHES "^${CURL_TEST}$") + IF(NOT DEFINED ${CURL_TEST}) SET(MACRO_CHECK_FUNCTION_DEFINITIONS "-D${CURL_TEST} ${CMAKE_REQUIRED_FLAGS}") IF(CMAKE_REQUIRED_LIBRARIES) @@ -533,7 +533,7 @@ MACRO(CURL_INTERNAL_TEST CURL_TEST) "Performing Curl Test ${CURL_TEST} failed with the following output:\n" "${OUTPUT}\n") ENDIF(${CURL_TEST}) - ENDIF("${CURL_TEST}" MATCHES "^${CURL_TEST}$") + ENDIF() ENDMACRO(CURL_INTERNAL_TEST) # Do curl specific tests -- cgit v0.12 From e177e7affb10fc25b71d4c9d9796c9df7fcdb800 Mon Sep 17 00:00:00 2001 From: Brad King Date: Mon, 8 Sep 2014 11:20:27 -0400 Subject: FPHSA: Avoid if() dereferencing of quoted variable Legacy invocations may pass a variable name where "DEFAULT_MSG" belongs. When comparing FPHSA_FAIL_MESSAGE to "DEFAULT_MSG", use a leading "x" on both sides to avoid mistaking the value of the message for a variable name. --- Modules/FindPackageHandleStandardArgs.cmake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/FindPackageHandleStandardArgs.cmake b/Modules/FindPackageHandleStandardArgs.cmake index d030418..e8d1dfb 100644 --- a/Modules/FindPackageHandleStandardArgs.cmake +++ b/Modules/FindPackageHandleStandardArgs.cmake @@ -201,7 +201,7 @@ function(FIND_PACKAGE_HANDLE_STANDARD_ARGS _NAME _FIRST_ARG) # now that we collected all arguments, process them - if("${FPHSA_FAIL_MESSAGE}" STREQUAL "DEFAULT_MSG") + if("x${FPHSA_FAIL_MESSAGE}" STREQUAL "xDEFAULT_MSG") set(FPHSA_FAIL_MESSAGE "Could NOT find ${_NAME}") endif() -- cgit v0.12 From 858d5a0b3e52dbae635fac5a6944fba23a362f5b Mon Sep 17 00:00:00 2001 From: Brad King Date: Tue, 9 Sep 2014 09:09:57 -0400 Subject: Fix if() checks of CMAKE_SYSTEM_NAME on Cygwin The CMAKE_SYSTEM_NAME is "CYGWIN", but we also define a variable named "CYGWIN" to "1". Avoid allowing if() to expand the "CYGWIN" string as a variable. --- CMakeCPack.cmake | 2 +- CMakeLists.txt | 2 +- Source/CMakeLists.txt | 2 +- Tests/CMakeLists.txt | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/CMakeCPack.cmake b/CMakeCPack.cmake index 66ec900..22ca8cf 100644 --- a/CMakeCPack.cmake +++ b/CMakeCPack.cmake @@ -50,7 +50,7 @@ if(EXISTS "${CMAKE_ROOT}/Modules/CPack.cmake") if(NOT DEFINED CPACK_SYSTEM_NAME) # make sure package is not Cygwin-unknown, for Cygwin just # cygwin is good for the system name - if("${CMAKE_SYSTEM_NAME}" STREQUAL "CYGWIN") + if("x${CMAKE_SYSTEM_NAME}" STREQUAL "xCYGWIN") set(CPACK_SYSTEM_NAME Cygwin) else() set(CPACK_SYSTEM_NAME ${CMAKE_SYSTEM_NAME}-${CMAKE_SYSTEM_PROCESSOR}) diff --git a/CMakeLists.txt b/CMakeLists.txt index 54aad83..14f60ae 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -381,7 +381,7 @@ macro (CMAKE_BUILD_UTILITIES) # Use curses? if (UNIX) # there is a bug in the Syllable libraries which makes linking ccmake fail, Alex - if(NOT "${CMAKE_SYSTEM_NAME}" MATCHES syllable) + if(NOT CMAKE_SYSTEM_NAME MATCHES syllable) set(CURSES_NEED_NCURSES TRUE) find_package(Curses QUIET) if (CURSES_LIBRARY) diff --git a/Source/CMakeLists.txt b/Source/CMakeLists.txt index c3f77f4..f9405b3 100644 --- a/Source/CMakeLists.txt +++ b/Source/CMakeLists.txt @@ -451,7 +451,7 @@ if (WIN32) endif () # Watcom support -if(WIN32 OR "${CMAKE_SYSTEM_NAME}" STREQUAL "Linux") +if(WIN32 OR CMAKE_SYSTEM_NAME STREQUAL "Linux") set_property(SOURCE cmake.cxx APPEND PROPERTY COMPILE_DEFINITIONS CMAKE_USE_WMAKE) list(APPEND SRCS cmGlobalWatcomWMakeGenerator.cxx diff --git a/Tests/CMakeLists.txt b/Tests/CMakeLists.txt index 7242a00..128498c 100644 --- a/Tests/CMakeLists.txt +++ b/Tests/CMakeLists.txt @@ -1392,7 +1392,7 @@ ${CMake_BINARY_DIR}/bin/cmake -DDIR=dev -P ${CMake_SOURCE_DIR}/Utilities/Release ) list(APPEND TEST_BUILD_DIRS "${CMake_BINARY_DIR}/Tests/Dependency") - if("${CMAKE_SYSTEM_NAME}" MATCHES syllable) + if(CMAKE_SYSTEM_NAME MATCHES syllable) # RPATH isn't supported under Syllable, so the tests don't # find their libraries. In order to fix that LIBRARY_OUTPUT_DIR -- cgit v0.12