ENH: New format for warning and error messages

- Add cmMakefile methods IssueError and IssueWarning
  - Maintain an explicit call stack in cmMakefile
  - Include context/call-stack info in messages
  - Nested errors now unwind the call stack
  - Use new mechanism for policy warnings and errors
  - Improve policy error message
  - Include cmExecutionStatus pointer in call stack
    so that errors deeper in the C++ stack under
    a command invocation will become errors for the
    command

Source/cmAddCustomTargetCommand.cxx

@@ -18,7 +18,8 @@
 
 // cmAddCustomTargetCommand
 bool cmAddCustomTargetCommand
-::InitialPass(std::vector<std::string> const& args, cmExecutionStatus &)
+::InitialPass(std::vector<std::string> const& args,
+              cmExecutionStatus& status)
 {
   // This enum must be before an enum is used in a switch statment. 
   // If not there is an ICE on the itanium version of gcc we are running
@@ -45,9 +46,9 @@ bool cmAddCustomTargetCommand
     switch (this->Makefile->GetPolicyStatus(cmPolicies::CMP_0001))
     {
       case cmPolicies::WARN:
-        cmSystemTools::Message(
+        this->Makefile->IssueWarning(
           this->Makefile->GetPolicies()->GetPolicyWarning
-            (cmPolicies::CMP_0001).c_str(),"Warning");
+            (cmPolicies::CMP_0001));
       case cmPolicies::OLD:
 //        if (this->Makefile->IsBWCompatibilityLessThan(2,2))
 //        {
@@ -60,9 +61,10 @@ bool cmAddCustomTargetCommand
         return false;
       case cmPolicies::REQUIRED_IF_USED:
       case cmPolicies::REQUIRED_ALWAYS:
-        this->SetError(
+        this->Makefile->IssueError(
           this->Makefile->GetPolicies()->GetRequiredPolicyError
-            (cmPolicies::CMP_0001).c_str());
+            (cmPolicies::CMP_0001).c_str()
+          );
         return false;
     }
   }

Source/cmExecutionStatus.h

@@ -42,12 +42,19 @@ public:
   { return this->BreakInvoked; }
             
   virtual void Clear()
-  { this->ReturnInvoked = false; this->BreakInvoked = false; }
+    {
+    this->ReturnInvoked = false;
+    this->BreakInvoked = false;
+    this->NestedError = false;
+    }
+  virtual void SetNestedError(bool val) { this->NestedError = val; }
+  virtual bool GetNestedError() { return this->NestedError; }
 
                                         
 protected:
   bool ReturnInvoked;
   bool BreakInvoked;
+  bool NestedError;
 };
 
 #endif

Source/cmFunctionCommand.cxx

@@ -86,7 +86,7 @@ public:
 
 bool cmFunctionHelperCommand::InvokeInitialPass
 (const std::vector<cmListFileArgument>& args,
- cmExecutionStatus &)
+ cmExecutionStatus & inStatus)
 {
   // Expand the argument list to the function.
   std::vector<std::string> expandedArgs;
@@ -157,18 +157,12 @@ bool cmFunctionHelperCommand::InvokeInitialPass
   for(unsigned int c = 0; c < this->Functions.size(); ++c)
     {
     cmExecutionStatus status;
-    if (!this->Makefile->ExecuteCommand(this->Functions[c],status))
+    if (!this->Makefile->ExecuteCommand(this->Functions[c],status) ||
+        status.GetNestedError())
       {
-      cmOStringStream error;
+      // The error message should have already included the call stack
-      error << "Error in cmake code at\n"
+      // so we do not need to report an error here.
-            << this->Functions[c].FilePath << ":" 
+      inStatus.SetNestedError(true);
-            << this->Functions[c].Line << ":\n"
-            << "A command failed during the invocation of function \""
-            << this->Args[0].c_str() << "\".";
-      cmSystemTools::Error(error.str().c_str());
-
-      // pop scope on the makefile and return
-      this->Makefile->PopScope();
       return false;
       }
     if (status.GetReturnInvoked())

Source/cmListFileCache.cxx

@@ -152,15 +152,15 @@ bool cmListFile::ParseFile(const char* filename,
       switch (mf->GetPolicyStatus(cmPolicies::CMP_0000))
       {
         case cmPolicies::WARN:
-          cmSystemTools::Message(
+          mf->IssueWarning(
-            mf->GetPolicies()->GetPolicyWarning
+            mf->GetPolicies()->GetPolicyWarning(cmPolicies::CMP_0000)
-              (cmPolicies::CMP_0000).c_str(),"Warning");
+            );
         case cmPolicies::OLD:
           break; 
         default:
-          cmSystemTools::Error(
+          mf->IssueError(
-            mf->GetPolicies()->GetRequiredPolicyError
+            mf->GetPolicies()->GetRequiredPolicyError(cmPolicies::CMP_0000)
-              (cmPolicies::CMP_0000).c_str());
+            );
           return false;
       }
     }

Source/cmListFileCache.h

@@ -50,14 +50,18 @@ struct cmListFileArgument
   long Line;
 };
 
-struct cmListFileFunction
+struct cmListFileContext
 {
   std::string Name;
-  std::vector<cmListFileArgument> Arguments;
   std::string FilePath;
   long Line;
 };
 
+struct cmListFileFunction: public cmListFileContext
+{
+  std::vector<cmListFileArgument> Arguments;
+};
+
 struct cmListFile
 {
   cmListFile() 

Source/cmMacroCommand.cxx

@@ -237,24 +237,12 @@ bool cmMacroHelperCommand::InvokeInitialPass
       newLFF.Arguments.push_back(arg);
       }
     cmExecutionStatus status;
-    if(!this->Makefile->ExecuteCommand(newLFF,status))
+    if(!this->Makefile->ExecuteCommand(newLFF, status) ||
+       status.GetNestedError())
       {
-      if(args.size())
+      // The error message should have already included the call stack
-        {
+      // so we do not need to report an error here.
-        arg.FilePath = args[0].FilePath;
+      inStatus.SetNestedError(true);
-        arg.Line = args[0].Line;
-        }
-      else
-        {
-        arg.FilePath =  "Unknown";
-        arg.Line = 0;
-        }
-      cmOStringStream error;
-      error << "Error in cmake code at\n"
-            << arg.FilePath << ":" << arg.Line << ":\n"
-            << "A command failed during the invocation of macro \""
-            << this->Args[0].c_str() << "\".";
-      cmSystemTools::Error(error.str().c_str());
       return false;
       }
     if (status.GetReturnInvoked())

Source/cmMakefile.cxx

@@ -280,6 +280,134 @@ bool cmMakefile::CommandExists(const char* name) const
   return this->GetCMakeInstance()->CommandExists(name);
 }
 
+//----------------------------------------------------------------------------
+// Helper function to print a block of text with every line following
+// a given prefix.
+void cmMakefilePrintPrefixed(std::ostream& os,  const char* prefix,
+                             std::string const& msg)
+{
+  bool newline = true;
+  for(const char* c = msg.c_str(); *c; ++c)
+    {
+    if(newline)
+      {
+      os << prefix;
+      newline = false;
+      }
+    os << *c;
+    if(*c == '\n')
+      {
+      newline = true;
+      }
+    }
+  if(!newline)
+    {
+    os << "\n";
+    }
+}
+
+//----------------------------------------------------------------------------
+void cmMakefile::IssueError(std::string const& msg) const
+{
+  this->IssueMessage(msg, true);
+}
+
+//----------------------------------------------------------------------------
+void cmMakefile::IssueWarning(std::string const& msg) const
+{
+  this->IssueMessage(msg, false);
+}
+
+//----------------------------------------------------------------------------
+void cmMakefile::IssueMessage(std::string const& text, bool isError) const
+{
+  cmOStringStream msg;
+
+  // Construct the message header.
+  if(isError)
+    {
+    msg << "CMake Error:";
+    }
+  else
+    {
+    msg << "CMake Warning:";
+    }
+
+  // Add the immediate context.
+  CallStackType::const_reverse_iterator i = this->CallStack.rbegin();
+  if(i != this->CallStack.rend())
+    {
+    if(isError)
+      {
+      i->Status->SetNestedError(true);
+      }
+    cmListFileContext const& lfc = *i->Context;
+    msg
+      << " at "
+      << this->LocalGenerator->Convert(lfc.FilePath.c_str(),
+                                       cmLocalGenerator::HOME)
+      << ":" << lfc.Line << " " << lfc.Name;
+    ++i;
+    }
+
+  // Add the message text.
+  msg << " {\n";
+  cmMakefilePrintPrefixed(msg, "  ", text);
+  msg << "}";
+
+  // Add the rest of the context.
+  if(i != this->CallStack.rend())
+    {
+    msg << " with call stack {\n";
+    while(i != this->CallStack.rend())
+      {
+      cmListFileContext const& lfc = *i->Context;
+      msg << "  "
+          << this->LocalGenerator->Convert(lfc.FilePath.c_str(),
+                                           cmLocalGenerator::HOME)
+          << ":" << lfc.Line << " " << lfc.Name << "\n";
+      ++i;
+      }
+    msg << "}\n";
+    }
+  else
+    {
+    msg << "\n";
+    }
+
+  // Output the message.
+  if(isError)
+    {
+    cmSystemTools::SetErrorOccured();
+    cmSystemTools::Message(msg.str().c_str(), "Error");
+    }
+  else
+    {
+    cmSystemTools::Message(msg.str().c_str(), "Warning");
+    }
+}
+
+//----------------------------------------------------------------------------
+// Helper class to make sure the call stack is valid.
+class cmMakefileCall
+{
+public:
+  cmMakefileCall(cmMakefile* mf,
+                 cmListFileContext const& lfc,
+                 cmExecutionStatus& status): Makefile(mf)
+    {
+    cmMakefile::CallStackEntry entry = {&lfc, &status};
+    this->Makefile->CallStack.push_back(entry);
+    }
+  ~cmMakefileCall()
+    {
+    this->Makefile->CallStack.pop_back();
+    }
+private:
+  cmMakefile* Makefile;
+};
+
+//----------------------------------------------------------------------------
 bool cmMakefile::ExecuteCommand(const cmListFileFunction& lff,
                                 cmExecutionStatus &status)
 {
@@ -294,34 +422,30 @@ bool cmMakefile::ExecuteCommand(const cmListFileFunction& lff,
   
   std::string name = lff.Name;
 
-  // execute the command
+  // Place this call on the call stack.
-  cmCommand *rm =
+  cmMakefileCall stack_manager(this, lff, status);
-    this->GetCMakeInstance()->GetCommand(name.c_str());
+  static_cast<void>(stack_manager);
-  if(rm)
+
+  // Lookup the command prototype.
+  if(cmCommand* proto = this->GetCMakeInstance()->GetCommand(name.c_str()))
     {
-    // const char* versionValue
+    // Clone the prototype.
-      // = this->GetDefinition("CMAKE_BACKWARDS_COMPATIBILITY");
+    cmsys::auto_ptr<cmCommand> pcmd(proto->Clone());
-    // int major = 0;
+    pcmd->SetMakefile(this);
-    // int minor = 0;
+
-    // if ( versionValue )
+    // Decide whether to invoke the command.
-      // {
+    if(pcmd->GetEnabled() && !cmSystemTools::GetFatalErrorOccured()  &&
-      // sscanf(versionValue, "%d.%d", &major, &minor);
+       (!this->GetCMakeInstance()->GetScriptMode() || pcmd->IsScriptable()))
-      // }
-    cmCommand* usedCommand = rm->Clone();
-    usedCommand->SetMakefile(this);
-    bool keepCommand = false;
-    if(usedCommand->GetEnabled() && !cmSystemTools::GetFatalErrorOccured()  &&
-       (!this->GetCMakeInstance()->GetScriptMode() ||
-        usedCommand->IsScriptable()))
       {
-      if(!usedCommand->InvokeInitialPass(lff.Arguments,status))
+      // Try invoking the command.
+      if(!pcmd->InvokeInitialPass(lff.Arguments,status) ||
+         status.GetNestedError())
         {
-        cmOStringStream error;
+        if(!status.GetNestedError())
-        error << "Error in cmake code at\n"
+          {
-              << lff.FilePath << ":" << lff.Line << ":\n"
+          // The command invocation requested that we report an error.
-              << usedCommand->GetError() << std::endl
+          this->IssueError(pcmd->GetError());
-              << "   Called from: " << this->GetListFileStack().c_str();
+          }
-        cmSystemTools::Error(error.str().c_str());
         result = false;
         if ( this->GetCMakeInstance()->GetScriptMode() )
           {
@@ -331,38 +455,28 @@ bool cmMakefile::ExecuteCommand(const cmListFileFunction& lff,
       else
         {
         // use the command
-        keepCommand = true;
+        this->UsedCommands.push_back(pcmd.release());
-        this->UsedCommands.push_back(usedCommand);
         }
       }
     else if ( this->GetCMakeInstance()->GetScriptMode()
-              && !usedCommand->IsScriptable() )
+              && !pcmd->IsScriptable() )
       {
-      cmOStringStream error;
+      std::string error = "Command ";
-      error << "Error in cmake code at\n"
+      error += pcmd->GetName();
-            << lff.FilePath << ":" << lff.Line << ":\n"
+      error += "() is not scriptable";
-            << "Command " << usedCommand->GetName()
+      this->IssueError(error);
-            << "() is not scriptable" << std::endl;
-      cmSystemTools::Error(error.str().c_str());
       result = false;
       cmSystemTools::SetFatalErrorOccured();
       }
-    // if the Cloned command was not used
-    // then delete it
-    if(!keepCommand)
-      {
-      delete usedCommand;
-      }
     }
   else
     {
     if(!cmSystemTools::GetFatalErrorOccured())
       {
-      cmOStringStream error;
+      std::string error = "Unknown CMake command \"";
-      error << "Error in cmake code at\n"
+      error += lff.Name;
-            << lff.FilePath << ":" << lff.Line << ":\n"
+      error += "\".";
-            << "Unknown CMake command \"" << lff.Name.c_str() << "\".";
+      this->IssueError(error);
-      cmSystemTools::Error(error.str().c_str());
       result = false;
       cmSystemTools::SetFatalErrorOccured();
       }
@@ -3152,8 +3266,8 @@ bool cmMakefile::EnforceUniqueName(std::string const& name, std::string& msg,
       switch (this->GetPolicyStatus(cmPolicies::CMP_0002))
         {
         case cmPolicies::WARN:
-          msg = this->GetPolicies()->
+          this->IssueWarning(this->GetPolicies()->
-            GetPolicyWarning(cmPolicies::CMP_0002);
+                             GetPolicyWarning(cmPolicies::CMP_0002));
         case cmPolicies::OLD:
           return true;
         case cmPolicies::REQUIRED_IF_USED:

Source/cmMakefile.h

@@ -41,6 +41,7 @@ class cmSourceFile;
 class cmTest;
 class cmVariableWatch;
 class cmake;
+class cmMakefileCall;
 
 /** \class cmMakefile
  * \brief Process the input CMakeLists.txt file.
@@ -783,6 +784,10 @@ public:
   void PopScope();
   void RaiseScope(const char *var, const char *value);
 
+  /** Issue messages with the given text plus context information.  */
+  void IssueWarning(std::string const& msg) const;
+  void IssueError(std::string const& msg) const;
+
 protected:
   // add link libraries and directories to the target
   void AddGlobalLinkInformation(const char* name, cmTarget& target);
@@ -876,6 +881,18 @@ private:
   // stack of list files being read 
   std::deque<cmStdString> ListFileStack;
 
+  // stack of commands being invoked.
+  struct CallStackEntry
+  {
+    cmListFileContext const* Context;
+    cmExecutionStatus* Status;
+  };
+  typedef std::deque<CallStackEntry> CallStackType;
+  CallStackType CallStack;
+  friend class cmMakefileCall;
+
+  void IssueMessage(std::string const& text, bool isError) const;
+
   cmTarget* FindBasicTarget(const char* name);
   std::vector<cmTarget*> ImportedTargetsOwned;
   std::map<cmStdString, cmTarget*> ImportedTargets;

Source/cmPolicies.cxx

@@ -86,7 +86,7 @@ cmPolicies::cmPolicies()
   // define all the policies
   this->DefinePolicy(CMP_0000, "CMP_0000",
     "Missing a CMake version specification. You must have a cmake_policy "
-    "or cmake_minimum_required call.",
+    "call.",
     "CMake requires that projects specify what version of CMake they have "
     "been written to. The easiest way to do this is by placing a call to "
     "cmake_policy at the top of your CMakeLists file. For example: "
@@ -348,10 +348,11 @@ std::string cmPolicies::GetPolicyWarning(cmPolicies::PolicyID id)
 
   cmOStringStream msg;
   msg <<
-    "WARNING: Policy " << pos->second->IDString << " is not set: "
+    "Policy " << pos->second->IDString << " is not set: "
-    "" << pos->second->ShortDescription << " "
+    "" << pos->second->ShortDescription << "\n"
     "Run \"cmake --help-policy " << pos->second->IDString << "\" for "
-    "policy details.  Use the cmake_policy command to set the policy "
+    "policy details.\n"
+    "Use the cmake_policy command to set the policy "
     "and suppress this warning.";
   return msg.str();
 }
@@ -371,20 +372,18 @@ std::string cmPolicies::GetRequiredPolicyError(cmPolicies::PolicyID id)
 
   cmOStringStream error;
   error <<
-    "Error " <<
+    "Policy " << pos->second->IDString << " is not set to NEW: "
-    pos->second->IDString << ": " <<
+    "" << pos->second->ShortDescription << "\n"
-    pos->second->ShortDescription <<
+    "Run \"cmake --help-policy " << pos->second->IDString << "\" for "
-    " This behavior is required now. You can suppress this message by "
+    "policy details.\n"
-    "specifying that your listfile is written to handle this new "
+    "CMake now requires this policy to be set to NEW by the project.  "
-    "behavior by adding either\n" <<
+    "The policy may be set explicitly using the code\n"
-    "cmake_policy (NEW " <<
+    "  cmake_policy(SET " << pos->second->IDString << " NEW)\n"
-    pos->second->IDString << ")\n or \n. " <<
+    "or by upgrading all policies with the code\n"
-    "cmake_policy (VERSION " << 
+    "  cmake_policy(VERSION " << pos->second->GetVersionString() <<
-    pos->second->GetVersionString() << " ) or later."
+    ") # or later\n"
-    "Run cmake --help-policy " <<
+    "Run \"cmake --help-command cmake_policy\" for more information.";
-    pos->second->IDString << " for more information.";
   return error.str();
-  
 }
 
 ///! Get the default status for a policy