Merge topic 'break-command-strictness'

d54617d0 break: Add policy CMP0055 to check calls strictly
bae604d9 Track nested loop levels in CMake language with a stack of counters

7 files changed, 145 insertions, 1 deletions

diff --git a/Source/cmBreakCommand.cxx b/Source/cmBreakCommand.cxx
index b70e400..ff527db 100644
--- a/Source/cmBreakCommand.cxx
+++ b/Source/cmBreakCommand.cxx
@@ -12,10 +12,76 @@
 #include "cmBreakCommand.h"
 
 // cmBreakCommand
-bool cmBreakCommand::InitialPass(std::vector<std::string> const&,
+bool cmBreakCommand::InitialPass(std::vector<std::string> const &args,
                                   cmExecutionStatus &status)
 {
+  if(!this->Makefile->IsLoopBlock())
+    {
+    bool issueMessage = true;
+    cmOStringStream e;
+    cmake::MessageType messageType = cmake::AUTHOR_WARNING;
+    switch(this->Makefile->GetPolicyStatus(cmPolicies::CMP0055))
+      {
+      case cmPolicies::WARN:
+        e << (this->Makefile->GetPolicies()
+                  ->GetPolicyWarning(cmPolicies::CMP0055)) << "\n";
+        break;
+      case cmPolicies::OLD:
+        issueMessage = false;
+        break;
+      case cmPolicies::REQUIRED_ALWAYS:
+      case cmPolicies::REQUIRED_IF_USED:
+      case cmPolicies::NEW:
+        messageType = cmake::FATAL_ERROR;
+        break;
+      }
+
+    if(issueMessage)
+      {
+      e << "A BREAK command was found outside of a proper "
+           "FOREACH or WHILE loop scope.";
+      this->Makefile->IssueMessage(messageType, e.str());
+       if(messageType == cmake::FATAL_ERROR)
+        {
+        return false;
+        }
+      }
+    }
+
   status.SetBreakInvoked(true);
+
+  if(!args.empty())
+    {
+    bool issueMessage = true;
+    cmOStringStream e;
+    cmake::MessageType messageType = cmake::AUTHOR_WARNING;
+    switch(this->Makefile->GetPolicyStatus(cmPolicies::CMP0055))
+      {
+      case cmPolicies::WARN:
+        e << (this->Makefile->GetPolicies()
+                  ->GetPolicyWarning(cmPolicies::CMP0055)) << "\n";
+        break;
+      case cmPolicies::OLD:
+        issueMessage = false;
+        break;
+      case cmPolicies::REQUIRED_ALWAYS:
+      case cmPolicies::REQUIRED_IF_USED:
+      case cmPolicies::NEW:
+        messageType = cmake::FATAL_ERROR;
+        break;
+      }
+
+    if(issueMessage)
+      {
+      e << "The BREAK command does not accept any arguments.";
+      this->Makefile->IssueMessage(messageType, e.str());
+       if(messageType == cmake::FATAL_ERROR)
+        {
+        return false;
+        }
+      }
+    }
+
   return true;
 }
 
diff --git a/Source/cmForEachCommand.cxx b/Source/cmForEachCommand.cxx
index e3f66c1..dec5157 100644
--- a/Source/cmForEachCommand.cxx
+++ b/Source/cmForEachCommand.cxx
@@ -27,6 +27,8 @@ IsFunctionBlocked(const cmListFileFunction& lff, cmMakefile &mf,
     // if this is the endofreach for this statement
     if (!this->Depth)
       {
+      cmMakefile::LoopBlockPop loopBlockPop(&mf);
+
       // Remove the function blocker for this scope or bail.
       cmsys::auto_ptr<cmFunctionBlocker>
         fb(mf.RemoveFunctionBlocker(this, lff));
@@ -73,6 +75,7 @@ IsFunctionBlocked(const cmListFileFunction& lff, cmMakefile &mf,
             }
           }
         }
+
       // restore the variable to its prior value
       mf.AddDefinition(this->Args[0],oldDef.c_str());
       return true;
@@ -199,6 +202,8 @@ bool cmForEachCommand
     }
   this->Makefile->AddFunctionBlocker(f);
 
+  this->Makefile->PushLoopBlock();
+
   return true;
 }
 
@@ -242,5 +247,8 @@ bool cmForEachCommand::HandleInMode(std::vector<std::string> const& args)
     }
 
   this->Makefile->AddFunctionBlocker(f.release()); // TODO: pass auto_ptr
+
+  this->Makefile->PushLoopBlock();
+
   return true;
 }
diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx
index 61807b2..6eca0f0 100644
--- a/Source/cmMakefile.cxx
+++ b/Source/cmMakefile.cxx
@@ -171,6 +171,9 @@ void cmMakefile::Initialize()
   // Protect the directory-level policies.
   this->PushPolicyBarrier();
 
+  // push empty loop block
+  this->PushLoopBlockBarrier();
+
   // By default the check is not done.  It is enabled by
   // cmListFileCache in the top level if necessary.
   this->CheckCMP0000 = false;
@@ -3354,6 +3357,38 @@ void cmMakefile::PopFunctionBlockerBarrier(bool reportError)
 }
 
 //----------------------------------------------------------------------------
+void cmMakefile::PushLoopBlock()
+{
+  assert(!this->LoopBlockCounter.empty());
+  this->LoopBlockCounter.top()++;
+}
+
+void cmMakefile::PopLoopBlock()
+{
+  assert(!this->LoopBlockCounter.empty());
+  assert(this->LoopBlockCounter.top() > 0);
+  this->LoopBlockCounter.top()--;
+}
+
+void cmMakefile::PushLoopBlockBarrier()
+{
+  this->LoopBlockCounter.push(0);
+}
+
+void cmMakefile::PopLoopBlockBarrier()
+{
+  assert(!this->LoopBlockCounter.empty());
+  assert(this->LoopBlockCounter.top() == 0);
+  this->LoopBlockCounter.pop();
+}
+
+bool cmMakefile::IsLoopBlock() const
+{
+  assert(!this->LoopBlockCounter.empty());
+  return !this->LoopBlockCounter.empty() && this->LoopBlockCounter.top() > 0;
+}
+
+//----------------------------------------------------------------------------
 bool cmMakefile::ExpandArguments(
   std::vector<cmListFileArgument> const& inArgs,
   std::vector<std::string>& outArgs) const
@@ -4487,10 +4522,14 @@ void cmMakefile::PushScope()
   this->Internal->VarStack.push(cmDefinitions(parent));
   this->Internal->VarInitStack.push(init);
   this->Internal->VarUsageStack.push(usage);
+
+  this->PushLoopBlockBarrier();
 }
 
 void cmMakefile::PopScope()
 {
+  this->PopLoopBlockBarrier();
+
   cmDefinitions* current = &this->Internal->VarStack.top();
   std::set<std::string> init = this->Internal->VarInitStack.top();
   std::set<std::string> usage = this->Internal->VarUsageStack.top();
diff --git a/Source/cmMakefile.h b/Source/cmMakefile.h
index 0458d54..41dddc3 100644
--- a/Source/cmMakefile.h
+++ b/Source/cmMakefile.h
@@ -34,6 +34,8 @@
 # include <cmsys/hash_map.hxx>
 #endif
 
+#include <stack>
+
 class cmFunctionBlocker;
 class cmCommand;
 class cmInstallGenerator;
@@ -123,6 +125,15 @@ public:
   };
   friend class LexicalPushPop;
 
+  class LoopBlockPop
+  {
+  public:
+    LoopBlockPop(cmMakefile* mf) { this->Makefile = mf; }
+    ~LoopBlockPop() { this->Makefile->PopLoopBlock(); }
+  private:
+    cmMakefile* Makefile;
+  };
+
   /**
    * Try running cmake and building a file. This is used for dynalically
    * loaded commands, not as part of the usual build process.
@@ -900,6 +911,10 @@ public:
   void PopScope();
   void RaiseScope(const std::string& var, const char *value);
 
+  // push and pop loop scopes
+  void PushLoopBlockBarrier();
+  void PopLoopBlockBarrier();
+
   /** Helper class to push and pop scopes automatically.  */
   class ScopePushPop
   {
@@ -960,6 +975,10 @@ public:
   void ClearMatches();
   void StoreMatches(cmsys::RegularExpression& re);
 
+  void PushLoopBlock();
+  void PopLoopBlock();
+  bool IsLoopBlock() const;
+
 protected:
   // add link libraries and directories to the target
   void AddGlobalLinkInformation(const std::string& name, cmTarget& target);
@@ -1054,6 +1073,8 @@ private:
   void PushFunctionBlockerBarrier();
   void PopFunctionBlockerBarrier(bool reportError = true);
 
+  std::stack<int> LoopBlockCounter;
+
   typedef std::map<std::string, std::string> StringStringMap;
   StringStringMap MacrosMap;
 
diff --git a/Source/cmPolicies.cxx b/Source/cmPolicies.cxx
index a420f59..64b87b7 100644
--- a/Source/cmPolicies.cxx
+++ b/Source/cmPolicies.cxx
@@ -364,6 +364,11 @@ cmPolicies::cmPolicies()
     CMP0054, "CMP0054",
     "Only interpret if() arguments as variables or keywords when unquoted.",
     3,1,0, cmPolicies::WARN);
+
+  this->DefinePolicy(
+    CMP0055, "CMP0055",
+    "Strict checking for break() command.",
+    3,2,0, cmPolicies::WARN);
 }
 
 cmPolicies::~cmPolicies()
diff --git a/Source/cmPolicies.h b/Source/cmPolicies.h
index 7c73da8..46ecc22 100644
--- a/Source/cmPolicies.h
+++ b/Source/cmPolicies.h
@@ -111,6 +111,7 @@ public:
     CMP0053, ///< Simplify variable reference and escape sequence evaluation
     CMP0054, ///< Only interpret if() arguments as variables
     /// or keywords when unquoted.
+    CMP0055, ///< Strict checking for break() command.
 
     /** \brief Always the last entry.
      *
diff --git a/Source/cmWhileCommand.cxx b/Source/cmWhileCommand.cxx
index 851c4cb..d36095e 100644
--- a/Source/cmWhileCommand.cxx
+++ b/Source/cmWhileCommand.cxx
@@ -27,6 +27,8 @@ IsFunctionBlocked(const cmListFileFunction& lff, cmMakefile &mf,
     // if this is the endwhile for this while loop then execute
     if (!this->Depth)
       {
+      cmMakefile::LoopBlockPop loopBlockPop(&mf);
+
       // Remove the function blocker for this scope or bail.
       cmsys::auto_ptr<cmFunctionBlocker>
         fb(mf.RemoveFunctionBlocker(this, lff));
@@ -138,6 +140,8 @@ bool cmWhileCommand
   f->Args = args;
   this->Makefile->AddFunctionBlocker(f);
 
+  this->Makefile->PushLoopBlock();
+
   return true;
 }