ENH: Improve argument parsing error messages
Previously error messages produced by parsing of command argument variable references, such as bad $KEY{VAR} syntax or a bad escape sequence, did not provide good context information. Errors parsing arguments inside macro invocations gave no context at all. Furthermore, some errors such as a missing close curly "${VAR" would be reported but build files would still be generated. These changes teach CMake to report errors with good context information for all command argument parsing problems. Policy CMP0010 is introduced so that existing projects that built despite such errors will continue to work.
This commit is contained in:
parent
4a1317de36
commit
d524f3675e
|
@ -87,8 +87,10 @@ char* cmCommandArgumentParserHelper::ExpandSpecialVariable(const char* key,
|
||||||
}
|
}
|
||||||
return this->EmptyVariable;
|
return this->EmptyVariable;
|
||||||
}
|
}
|
||||||
cmSystemTools::Error("Key ", key,
|
cmOStringStream e;
|
||||||
" is not used yet. For now only $ENV{..} is allowed");
|
e << "Syntax $" << key << "{} is not supported. "
|
||||||
|
<< "Only ${} and ENV{} are allowed.";
|
||||||
|
this->SetError(e.str());
|
||||||
return 0;
|
return 0;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -216,10 +218,11 @@ bool cmCommandArgumentParserHelper::HandleEscapeSymbol
|
||||||
this->AllocateParserType(pt, "\0", 1);
|
this->AllocateParserType(pt, "\0", 1);
|
||||||
break;
|
break;
|
||||||
default:
|
default:
|
||||||
char buffer[2];
|
{
|
||||||
buffer[0] = symbol;
|
cmOStringStream e;
|
||||||
buffer[1] = 0;
|
e << "Invalid escape sequence \\" << symbol;
|
||||||
cmSystemTools::Error("Invalid escape sequence \\", buffer);
|
this->SetError(e.str());
|
||||||
|
}
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
return true;
|
return true;
|
||||||
|
@ -300,7 +303,7 @@ void cmCommandArgumentParserHelper::Error(const char* str)
|
||||||
unsigned long pos = static_cast<unsigned long>(this->InputBufferPos);
|
unsigned long pos = static_cast<unsigned long>(this->InputBufferPos);
|
||||||
cmOStringStream ostr;
|
cmOStringStream ostr;
|
||||||
ostr << str << " (" << pos << ")";
|
ostr << str << " (" << pos << ")";
|
||||||
this->ErrorString = ostr.str();
|
this->SetError(ostr.str());
|
||||||
}
|
}
|
||||||
|
|
||||||
void cmCommandArgumentParserHelper::SetMakefile(const cmMakefile* mf)
|
void cmCommandArgumentParserHelper::SetMakefile(const cmMakefile* mf)
|
||||||
|
@ -318,3 +321,11 @@ void cmCommandArgumentParserHelper::SetResult(const char* value)
|
||||||
this->Result = value;
|
this->Result = value;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
void cmCommandArgumentParserHelper::SetError(std::string const& msg)
|
||||||
|
{
|
||||||
|
// Keep only the first error.
|
||||||
|
if(this->ErrorString.empty())
|
||||||
|
{
|
||||||
|
this->ErrorString = msg;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
|
@ -95,6 +95,7 @@ private:
|
||||||
char* AddString(const char* str);
|
char* AddString(const char* str);
|
||||||
|
|
||||||
void CleanupParser();
|
void CleanupParser();
|
||||||
|
void SetError(std::string const& msg);
|
||||||
|
|
||||||
std::vector<char*> Variables;
|
std::vector<char*> Variables;
|
||||||
const cmMakefile* Makefile;
|
const cmMakefile* Makefile;
|
||||||
|
|
|
@ -2168,28 +2168,60 @@ const char *cmMakefile::ExpandVariablesInString(std::string& source,
|
||||||
parser.SetReplaceAtSyntax(replaceAt);
|
parser.SetReplaceAtSyntax(replaceAt);
|
||||||
parser.SetRemoveEmpty(removeEmpty);
|
parser.SetRemoveEmpty(removeEmpty);
|
||||||
int res = parser.ParseString(source.c_str(), 0);
|
int res = parser.ParseString(source.c_str(), 0);
|
||||||
if ( res )
|
const char* emsg = parser.GetError();
|
||||||
|
if ( res && !emsg[0] )
|
||||||
{
|
{
|
||||||
source = parser.GetResult();
|
source = parser.GetResult();
|
||||||
}
|
}
|
||||||
else
|
else
|
||||||
{
|
{
|
||||||
|
// Construct the main error message.
|
||||||
cmOStringStream error;
|
cmOStringStream error;
|
||||||
error << "Syntax error in cmake code at\n"
|
error << "Syntax error in cmake code ";
|
||||||
<< (filename?filename:"(no filename given)")
|
if(filename && line > 0)
|
||||||
<< ":" << line << ":\n"
|
|
||||||
<< parser.GetError() << ", when parsing string \""
|
|
||||||
<< source.c_str() << "\"";
|
|
||||||
if(this->NeedBackwardsCompatibility(2,0))
|
|
||||||
{
|
{
|
||||||
cmSystemTools::Error(error.str().c_str());
|
// This filename and line number may be more specific than the
|
||||||
cmSystemTools::SetFatalErrorOccured();
|
// command context because one command invocation can have
|
||||||
return source.c_str();
|
// arguments on multiple lines.
|
||||||
|
error << "at\n"
|
||||||
|
<< " " << filename << ":" << line << "\n";
|
||||||
}
|
}
|
||||||
else
|
error << "when parsing string\n"
|
||||||
|
<< " " << source.c_str() << "\n";
|
||||||
|
error << emsg;
|
||||||
|
|
||||||
|
// If the parser failed ("res" is false) then this is a real
|
||||||
|
// argument parsing error, so the policy applies. Otherwise the
|
||||||
|
// parser reported an error message without failing because the
|
||||||
|
// helper implementation is unhappy, which has always reported an
|
||||||
|
// error.
|
||||||
|
cmake::MessageType mtype = cmake::FATAL_ERROR;
|
||||||
|
if(!res)
|
||||||
{
|
{
|
||||||
cmSystemTools::Message(error.str().c_str());
|
// This is a real argument parsing error. Use policy CMP0010 to
|
||||||
|
// decide whether it is an error.
|
||||||
|
switch(this->GetPolicyStatus(cmPolicies::CMP0010))
|
||||||
|
{
|
||||||
|
case cmPolicies::WARN:
|
||||||
|
error << "\n"
|
||||||
|
<< (this->GetPolicies()
|
||||||
|
->GetPolicyWarning(cmPolicies::CMP0010));
|
||||||
|
case cmPolicies::OLD:
|
||||||
|
// OLD behavior is to just warn and continue.
|
||||||
|
mtype = cmake::AUTHOR_WARNING;
|
||||||
|
break;
|
||||||
|
case cmPolicies::REQUIRED_IF_USED:
|
||||||
|
case cmPolicies::REQUIRED_ALWAYS:
|
||||||
|
error << "\n"
|
||||||
|
<< (this->GetPolicies()
|
||||||
|
->GetRequiredPolicyError(cmPolicies::CMP0010));
|
||||||
|
case cmPolicies::NEW:
|
||||||
|
// NEW behavior is to report the error.
|
||||||
|
cmSystemTools::SetFatalErrorOccured();
|
||||||
|
break;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
this->IssueMessage(mtype, error.str());
|
||||||
}
|
}
|
||||||
return source.c_str();
|
return source.c_str();
|
||||||
}
|
}
|
||||||
|
|
|
@ -323,6 +323,18 @@ cmPolicies::cmPolicies()
|
||||||
"by default, but only if FOLLOW_SYMLINKS is given as an additional "
|
"by default, but only if FOLLOW_SYMLINKS is given as an additional "
|
||||||
"argument to the FILE command.",
|
"argument to the FILE command.",
|
||||||
2,6,2, cmPolicies::WARN);
|
2,6,2, cmPolicies::WARN);
|
||||||
|
|
||||||
|
this->DefinePolicy(
|
||||||
|
CMP0010, "CMP0010",
|
||||||
|
"Bad variable reference syntax is an error.",
|
||||||
|
"In CMake 2.6.2 and below, incorrect variable reference syntax such as "
|
||||||
|
"a missing close-brace (\"${FOO\") was reported but did not stop "
|
||||||
|
"processing of CMake code. "
|
||||||
|
"This policy determines whether a bad variable reference is an error. "
|
||||||
|
"The OLD behavior for this policy is to warn about the error, leave "
|
||||||
|
"the string untouched, and continue. "
|
||||||
|
"The NEW behavior for this policy is to report an error.",
|
||||||
|
2,6,3, cmPolicies::WARN);
|
||||||
}
|
}
|
||||||
|
|
||||||
cmPolicies::~cmPolicies()
|
cmPolicies::~cmPolicies()
|
||||||
|
|
|
@ -50,6 +50,7 @@ public:
|
||||||
CMP0007, // list command handling of empty elements
|
CMP0007, // list command handling of empty elements
|
||||||
CMP0008, // Full-path libraries must be a valid library file name
|
CMP0008, // Full-path libraries must be a valid library file name
|
||||||
CMP0009, // GLOB_RECURSE should not follow symlinks by default
|
CMP0009, // GLOB_RECURSE should not follow symlinks by default
|
||||||
|
CMP0010, // Bad variable reference syntax is an error
|
||||||
|
|
||||||
// Always the last entry. Useful mostly to avoid adding a comma
|
// Always the last entry. Useful mostly to avoid adding a comma
|
||||||
// the last policy when adding a new one.
|
// the last policy when adding a new one.
|
||||||
|
|
Loading…
Reference in New Issue