ENH: modified the if command to address bug 9123 some

This commit is contained in:
Ken Martin 2009-06-12 10:07:05 -04:00
parent 7e03edf1df
commit a73071ca17
5 changed files with 341 additions and 117 deletions

View File

@ -9,8 +9,8 @@
Copyright (c) 2002 Kitware, Inc., Insight Consortium. All rights reserved.
See Copyright.txt or http://www.cmake.org/HTML/Copyright.html for details.
This software is distributed WITHOUT ANY WARRANTY; without even
the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR
This software is distributed WITHOUT ANY WARRANTY; without even
the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR
PURPOSE. See the above copyright notices for more information.
=========================================================================*/
@ -23,7 +23,7 @@
//=========================================================================
bool cmIfFunctionBlocker::
IsFunctionBlocked(const cmListFileFunction& lff,
IsFunctionBlocked(const cmListFileFunction& lff,
cmMakefile &mf,
cmExecutionStatus &inStatus)
{
@ -36,7 +36,7 @@ IsFunctionBlocked(const cmListFileFunction& lff,
{
this->ScopeDepth--;
// if this is the endif for this if statement, then start executing
if (!this->ScopeDepth)
if (!this->ScopeDepth)
{
// Remove the function blocker for this scope or bail.
cmsys::auto_ptr<cmFunctionBlocker>
@ -78,13 +78,16 @@ IsFunctionBlocked(const cmListFileFunction& lff,
static_cast<void>(stack_manager);
std::string errorString;
std::vector<std::string> expandedArguments;
mf.ExpandArguments(this->Functions[c].Arguments,
mf.ExpandArguments(this->Functions[c].Arguments,
expandedArguments);
bool isTrue =
cmIfCommand::IsTrue(expandedArguments,errorString,&mf);
cmake::MessageType status;
bool isTrue =
cmIfCommand::IsTrue(expandedArguments, errorString,
&mf, status);
if (errorString.size())
{
std::string err = "had incorrect arguments: ";
@ -99,11 +102,14 @@ IsFunctionBlocked(const cmListFileFunction& lff,
err += "(";
err += errorString;
err += ").";
mf.IssueMessage(cmake::FATAL_ERROR, err);
cmSystemTools::SetFatalErrorOccured();
return true;
mf.IssueMessage(status, err);
if (status == cmake::FATAL_ERROR)
{
cmSystemTools::SetFatalErrorOccured();
return true;
}
}
if (isTrue)
{
this->IsBlocking = false;
@ -111,7 +117,7 @@ IsFunctionBlocked(const cmListFileFunction& lff,
}
}
}
// should we execute?
else if (!this->IsBlocking)
{
@ -132,10 +138,10 @@ IsFunctionBlocked(const cmListFileFunction& lff,
return true;
}
}
// record the command
this->Functions.push_back(lff);
// always return true
return true;
}
@ -160,16 +166,19 @@ bool cmIfFunctionBlocker::ShouldRemove(const cmListFileFunction& lff,
//=========================================================================
bool cmIfCommand
::InvokeInitialPass(const std::vector<cmListFileArgument>& args,
::InvokeInitialPass(const std::vector<cmListFileArgument>& args,
cmExecutionStatus &)
{
std::string errorString;
std::vector<std::string> expandedArguments;
this->Makefile->ExpandArguments(args, expandedArguments);
bool isTrue =
cmIfCommand::IsTrue(expandedArguments,errorString,this->Makefile);
cmake::MessageType status;
bool isTrue =
cmIfCommand::IsTrue(expandedArguments,errorString,
this->Makefile, status);
if (errorString.size())
{
std::string err = "had incorrect arguments: ";
@ -184,11 +193,18 @@ bool cmIfCommand
err += "(";
err += errorString;
err += ").";
this->SetError(err.c_str());
cmSystemTools::SetFatalErrorOccured();
return false;
if (status == cmake::FATAL_ERROR)
{
this->SetError(err.c_str());
cmSystemTools::SetFatalErrorOccured();
return false;
}
else
{
this->Makefile->IssueMessage(status, err);
}
}
cmIfFunctionBlocker *f = new cmIfFunctionBlocker();
// if is isn't true block the commands
f->ScopeDepth = 1;
@ -199,13 +215,115 @@ bool cmIfCommand
}
f->Args = args;
this->Makefile->AddFunctionBlocker(f);
return true;
}
namespace
namespace
{
//=========================================================================
//=========================================================================
// returns true if succesfull, the resulting bool parsed is stored in result
bool GetBooleanValue(std::string &newArg,
cmMakefile *makefile,
bool &result,
std::string &errorString,
cmPolicies::PolicyStatus Policy12Status,
cmake::MessageType &status)
{
if (Policy12Status != cmPolicies::OLD &&
Policy12Status != cmPolicies::WARN)
{
// please note IsOn(var) does not always equal !IsOff(var)
// that is why each is called
if (cmSystemTools::IsOn(newArg.c_str()))
{
result = true;
return true;
}
if (cmSystemTools::IsOff(newArg.c_str()))
{
result = false;
return true;
}
return false;
}
// Old policy is more complex...
// 0 and 1 are very common, test for them first quickly
if (newArg == "0")
{
result = false;
return true;
}
if (newArg == "1")
{
result = true;
return true;
}
// old behavior is to dereference the var
if (Policy12Status == cmPolicies::OLD)
{
return false;
}
// now test for values that may be the name of a variable
// warn if used
if (cmSystemTools::IsOn(newArg.c_str()))
{
// only warn if the value would change
const char *def = makefile->GetDefinition(newArg.c_str());
if (cmSystemTools::IsOff(def))
{
cmPolicies* policies = makefile->GetPolicies();
errorString = "You have used a variable or argument named \""
+ newArg
+ "\" in a conditional statement. Please be aware of issues "
+ "related to policy CMP0012. "
+ policies->GetPolicyWarning(cmPolicies::CMP0012);
status = cmake::AUTHOR_WARNING;
}
return false;
}
if (cmSystemTools::IsOff(newArg.c_str()))
{
// only warn if the value would change
const char *def = makefile->GetDefinition(newArg.c_str());
if (!cmSystemTools::IsOff(def))
{
cmPolicies* policies = makefile->GetPolicies();
errorString = "You have used a variable or argument named \""
+ newArg
+ "\" in a conditional statement. Please be aware of issues "
+ "related to policy CMP0012. "
+ policies->GetPolicyWarning(cmPolicies::CMP0012);
status = cmake::AUTHOR_WARNING;
}
return false;
}
return false;
}
//=========================================================================
// returns the resulting boolean value
bool GetBooleanValueWithAutoDereference(
std::string &newArg,
cmMakefile *makefile,
std::string &errorString,
cmPolicies::PolicyStatus Policy12Status,
cmake::MessageType &status)
{
bool result = false;
if (GetBooleanValue(newArg, makefile, result,
errorString, Policy12Status, status))
{
return result;
}
const char *def = makefile->GetDefinition(newArg.c_str());
return !cmSystemTools::IsOff(def);
}
//=========================================================================
void IncrementArguments(std::list<std::string> &newArgs,
std::list<std::string>::iterator &argP1,
std::list<std::string>::iterator &argP2)
@ -233,7 +351,7 @@ namespace
{
*arg = "1";
}
else
else
{
*arg = "0";
}
@ -255,7 +373,7 @@ namespace
{
*arg = "1";
}
else
else
{
*arg = "0";
}
@ -298,7 +416,8 @@ namespace
// level 0 processes parenthetical expressions
bool HandleLevel0(std::list<std::string> &newArgs,
cmMakefile *makefile,
std::string &errorString)
std::string &errorString,
cmake::MessageType &status)
{
int reducible;
do
@ -329,6 +448,7 @@ namespace
if (depth)
{
errorString = "mismatched parenthesis in condition";
status = cmake::FATAL_ERROR;
return false;
}
// store the reduced args in this vector
@ -338,26 +458,26 @@ namespace
std::list<std::string>::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);
bool value =
cmIfCommand::IsTrue(newArgs2, errorString, makefile, status);
if(value)
{
*arg = "1";
}
else
else
{
*arg = "0";
}
argP1 = arg;
argP1++;
// remove the now evaluated parenthetical expression
newArgs.erase(argP1,argClose);
newArgs.erase(argP1,argClose);
}
++arg;
}
@ -365,12 +485,12 @@ namespace
while (reducible);
return true;
}
//=========================================================================
// level one handles most predicates except for NOT
bool HandleLevel1(std::list<std::string> &newArgs,
cmMakefile *makefile,
std::string &)
std::string &, cmake::MessageType &)
{
int reducible;
do
@ -454,7 +574,8 @@ namespace
// level two handles most binary operations except for AND OR
bool HandleLevel2(std::list<std::string> &newArgs,
cmMakefile *makefile,
std::string &errorString)
std::string &errorString,
cmake::MessageType &status)
{
int reducible;
const char *def;
@ -470,7 +591,7 @@ namespace
argP1 = arg;
IncrementArguments(newArgs,argP1,argP2);
if (argP1 != newArgs.end() && argP2 != newArgs.end() &&
*(argP1) == "MATCHES")
*(argP1) == "MATCHES")
{
def = cmIfCommand::GetVariableOrString(arg->c_str(), makefile);
const char* rex = (argP2)->c_str();
@ -481,6 +602,7 @@ namespace
cmOStringStream error;
error << "Regular expression \"" << rex << "\" cannot compile";
errorString = error.str();
status = cmake::FATAL_ERROR;
return false;
}
if (regEntry.find(def))
@ -499,7 +621,7 @@ namespace
reducible = 1;
}
if (argP1 != newArgs.end() && *arg == "MATCHES")
if (argP1 != newArgs.end() && *arg == "MATCHES")
{
*arg = "0";
newArgs.erase(argP1);
@ -509,8 +631,8 @@ namespace
}
if (argP1 != newArgs.end() && argP2 != newArgs.end() &&
(*(argP1) == "LESS" || *(argP1) == "GREATER" ||
*(argP1) == "EQUAL"))
(*(argP1) == "LESS" || *(argP1) == "GREATER" ||
*(argP1) == "EQUAL"))
{
def = cmIfCommand::GetVariableOrString(arg->c_str(), makefile);
def2 = cmIfCommand::GetVariableOrString((argP2)->c_str(), makefile);
@ -529,19 +651,19 @@ namespace
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"))
(*(argP1) == "STRLESS" ||
*(argP1) == "STREQUAL" ||
*(argP1) == "STRGREATER"))
{
def = cmIfCommand::GetVariableOrString(arg->c_str(), makefile);
def2 = cmIfCommand::GetVariableOrString((argP2)->c_str(), makefile);
@ -607,10 +729,11 @@ namespace
// level 3 handles NOT
bool HandleLevel3(std::list<std::string> &newArgs,
cmMakefile *makefile,
std::string &)
std::string &errorString,
cmPolicies::PolicyStatus Policy12Status,
cmake::MessageType &status)
{
int reducible;
const char *def;
do
{
reducible = 0;
@ -623,9 +746,11 @@ namespace
IncrementArguments(newArgs,argP1,argP2);
if (argP1 != newArgs.end() && *arg == "NOT")
{
def = cmIfCommand::GetVariableOrNumber((argP1)->c_str(), makefile);
HandlePredicate(cmSystemTools::IsOff(def),
reducible, arg, newArgs, argP1, argP2);
bool rhs = GetBooleanValueWithAutoDereference(*argP1, makefile,
errorString,
Policy12Status,
status);
HandlePredicate(!rhs, reducible, arg, newArgs, argP1, argP2);
}
++arg;
}
@ -638,11 +763,13 @@ namespace
// level 4 handles AND OR
bool HandleLevel4(std::list<std::string> &newArgs,
cmMakefile *makefile,
std::string &)
std::string &errorString,
cmPolicies::PolicyStatus Policy12Status,
cmake::MessageType &status)
{
int reducible;
const char *def;
const char *def2;
bool lhs;
bool rhs;
do
{
reducible = 0;
@ -653,23 +780,33 @@ namespace
{
argP1 = arg;
IncrementArguments(newArgs,argP1,argP2);
if (argP1 != newArgs.end() && *(argP1) == "AND" &&
if (argP1 != newArgs.end() && *(argP1) == "AND" &&
argP2 != newArgs.end())
{
def = cmIfCommand::GetVariableOrNumber(arg->c_str(), makefile);
def2 = cmIfCommand::GetVariableOrNumber((argP2)->c_str(), makefile);
HandleBinaryOp(
!(cmSystemTools::IsOff(def) || cmSystemTools::IsOff(def2)),
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" &&
if (argP1 != newArgs.end() && *(argP1) == "OR" &&
argP2 != newArgs.end())
{
def = cmIfCommand::GetVariableOrNumber(arg->c_str(), makefile);
def2 = cmIfCommand::GetVariableOrNumber((argP2)->c_str(), makefile);
HandleBinaryOp(
!(cmSystemTools::IsOff(def) && cmSystemTools::IsOff(def2)),
lhs = GetBooleanValueWithAutoDereference(*arg, makefile,
errorString,
Policy12Status,
status);
rhs = GetBooleanValueWithAutoDereference(*argP2, makefile,
errorString,
Policy12Status,
status);
HandleBinaryOp((lhs || rhs),
reducible, arg, newArgs, argP1, argP2);
}
++arg;
@ -683,9 +820,9 @@ namespace
//=========================================================================
// 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
// 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
//
@ -699,9 +836,9 @@ namespace
bool cmIfCommand::IsTrue(const std::vector<std::string> &args,
std::string &errorString, cmMakefile *makefile)
std::string &errorString, cmMakefile *makefile,
cmake::MessageType &status)
{
const char *def;
errorString = "";
// handle empty invocation
@ -715,57 +852,57 @@ bool cmIfCommand::IsTrue(const std::vector<std::string> &args,
// 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
if (!HandleLevel0(newArgs, makefile, errorString)) // parens
// parens
if (!HandleLevel0(newArgs, makefile, errorString, status))
{
return false;
}
if (!HandleLevel1(newArgs, makefile, errorString)) //predicates
//predicates
if (!HandleLevel1(newArgs, makefile, errorString, status))
{
return false;
}
if (!HandleLevel2(newArgs, makefile, errorString)) // binary ops
// binary ops
if (!HandleLevel2(newArgs, makefile, errorString, status))
{
return false;
}
if (!HandleLevel3(newArgs, makefile, errorString)) // NOT
// 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;
}
if (!HandleLevel4(newArgs, makefile, errorString)) // AND OR
// 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)
{
if (*newArgs.begin() == "0")
{
return false;
}
if (*newArgs.begin() == "1")
{
return true;
}
def = makefile->GetDefinition(args[0].c_str());
if(cmSystemTools::IsOff(def))
{
return false;
}
}
else
if (newArgs.size() != 1)
{
errorString = "Unknown arguments specified";
return false;
}
return true;
return GetBooleanValueWithAutoDereference(*(newArgs.begin()),
makefile,
errorString,
Policy12Status,
status);
}
//=========================================================================
@ -779,18 +916,3 @@ const char* cmIfCommand::GetVariableOrString(const char* str,
}
return def;
}
//=========================================================================
const char* cmIfCommand::GetVariableOrNumber(const char* str,
const cmMakefile* mf)
{
const char* def = mf->GetDefinition(str);
if(!def)
{
if (atoi(str))
{
def = str;
}
}
return def;
}

View File

@ -191,6 +191,71 @@ public:
"examples. Where there are nested parenthesis the innermost are "
"evaluated as part of evaluating the expression "
"that contains them."
"\n"
"The if statement was written fairly early in CMake's history "
"and it has some convenience features that may be confusing for "
"new users. The if statement reduces operations until there is "
"a single remaining value, at that point if the case "
"insensitive value is: ON, 1, YES, TRUE, Y it returns true, if "
"it is OFF, 0, NO, FALSE, N, NOTFOUND, *-NOTFOUND, IGNORE it "
"will return false. \n"
"This is fairly reasonable. The convenience feature that makes "
"it more confusing is how CMake handles values that do not "
"match the true or false list. Those values are treated as "
"variables and are dereferenced even though they do not have "
"the required ${} syntax. This means that if you write\n"
" if (boobah)\n"
"CMake will treat it as if you wrote \n"
" if (${boobah})\n"
"likewise if you write \n"
" if (fubar AND sol)\n"
"CMake will conveniently treat it as \n"
" if (\"${fubar}\" AND \"${sol}\")\n"
"The later is really the correct way to write it, but the "
"former will work as well. Only some operations in the if "
"statement have this special handling of arguments. The "
"specific details follow: \n"
"1) The left hand argument to MATCHES is first checked to see "
"if it is a defined variable, if so the variable's value is "
"used, otherwise the original value is used. \n"
"2) If the left hand argument to MATCHES is missing it returns "
"false without error \n"
"3) Both left and right hand arguments to LESS GREATER EQUAL "
"are independently tested to see if they are defined variables, "
"if so their defined values are used otherwise the original "
"value is used. \n"
"4) Both left and right hand arguments to STRLESS STREQUAL "
"STRGREATER are independently tested to see if they are defined "
"variables, if so their defined values are used otherwise the "
"original value is used. \n"
"5) Both left and right hand argumemnts to VERSION_LESS "
"VERSION_EQUAL VERSION_GREATER are independently tested to see "
"if they are defined variables, if so their defined values are "
"used otherwise the original value is used. \n"
"6) The right hand argument to NOT is tested to see if it is a "
"boolean constant, if so the value is used, otherwise it is "
"assumed to be a variable and it is dereferenced. \n"
"7) The left and right hand arguments to 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. \n"
;
}
@ -198,15 +263,13 @@ public:
// 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<std::string> &args,
std::string &errorString, cmMakefile *mf);
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 char* str,
const cmMakefile* mf);
static const char* GetVariableOrNumber(const char* str,
const cmMakefile* mf);
cmTypeMacro(cmIfCommand, cmCommand);
};

View File

@ -355,6 +355,19 @@ cmPolicies::cmPolicies()
"The NEW behavior for this policy is to allow the commands to do their "
"default cmake_policy PUSH and POP.",
2,6,3, cmPolicies::WARN);
this->DefinePolicy(
CMP0012, "CMP0012",
"In CMake versions prior to 2.6.5 the only boolean constants were 0 and 1. "
"Other boolean constants such as true, false, yes, no, "
"on, off, y, n, notfound, ignore were recognized in some cases but not all. "
"In later versions of cmake these values are treated as boolean constants "
"more consistently and should not be used as variable names. "
"Please do not use them as variable names.",
"The OLD behavior for this policy is to allow variables to have names such as "
"true and to dereference them. "
"The NEW behavior for this policy is to treat strings like true as a boolean constant.",
2,6,5, cmPolicies::WARN);
}
cmPolicies::~cmPolicies()

View File

@ -52,6 +52,7 @@ public:
CMP0009, // GLOB_RECURSE should not follow symlinks by default
CMP0010, // Bad variable reference syntax is an error
CMP0011, // Strong policy scope for include and find_package
CMP0012, // Strong handling of boolean constants
// Always the last entry. Useful mostly to avoid adding a comma
// the last policy when adding a new one.

View File

@ -41,11 +41,35 @@ IsFunctionBlocked(const cmListFileFunction& lff, cmMakefile &mf,
std::vector<std::string> expandedArguments;
mf.ExpandArguments(this->Args, expandedArguments);
cmake::MessageType messageType;
bool isTrue =
cmIfCommand::IsTrue(expandedArguments,errorString,&mf);
cmIfCommand::IsTrue(expandedArguments,errorString,
&mf, messageType);
while (isTrue)
{
if (errorString.size())
{
std::string err = "had incorrect arguments: ";
unsigned int i;
for(i =0; i < this->Args.size(); ++i)
{
err += (this->Args[i].Quoted?"\"":"");
err += this->Args[i].Value;
err += (this->Args[i].Quoted?"\"":"");
err += " ";
}
err += "(";
err += errorString;
err += ").";
mf.IssueMessage(messageType, err);
if (messageType == cmake::FATAL_ERROR)
{
cmSystemTools::SetFatalErrorOccured();
return true;
}
}
// Invoke all the functions that were collected in the block.
for(unsigned int c = 0; c < this->Functions.size(); ++c)
{
@ -68,7 +92,8 @@ IsFunctionBlocked(const cmListFileFunction& lff, cmMakefile &mf,
expandedArguments.clear();
mf.ExpandArguments(this->Args, expandedArguments);
isTrue =
cmIfCommand::IsTrue(expandedArguments,errorString,&mf);
cmIfCommand::IsTrue(expandedArguments,errorString,
&mf, messageType);
}
return true;
}