Merge topic 'makefile-stack-manipulation'

dcaa52c6c3 PrintCallStack: "Fix" entry suppression
a73955ae76 cmMakefile: Allow arbitrary stack manipulation
de5700a15a cmMakefile: Prevent copying RAII helpers

Acked-by: Kitware Robot <[email protected]>
Merge-request: !10067

Source/cmListFileCache.h

@@ -138,10 +138,10 @@ public:
 
   static cmListFileContext FromListFilePath(std::string const& filePath)
   {
-    // We are entering a file-level scope but have not yet reached
-    // any specific line or command invocation within it.  This context
-    // is useful to print when it is at the top but otherwise can be
-    // skipped during call stack printing.
+    // We are entering a file-level scope but have not yet reached any specific
+    // line or command invocation within it.  This context is useful to print
+    // when it is at the top, but otherwise can be skipped during call stack
+    // printing if preceded by a more specific entry.
     cmListFileContext lfc;
     lfc.FilePath = filePath;
     return lfc;

Source/cmMakefile.cxx

@@ -400,20 +400,59 @@ void cmMakefile::PrintCommandTrace(cmListFileFunction const& lff,
   }
 }
 
+cmMakefile::CallRAII::CallRAII(cmMakefile* mf, std::string const& file,
+                               cmExecutionStatus& status)
+  : CallRAII(mf, cmListFileContext::FromListFilePath(file), status)
+{
+}
+
+cmMakefile::CallRAII::CallRAII(cmMakefile* mf, const cmListFileContext& lfc,
+                               cmExecutionStatus& status)
+  : Makefile{ mf }
+{
+  this->Makefile->Backtrace = this->Makefile->Backtrace.Push(lfc);
+  ++this->Makefile->RecursionDepth;
+  this->Makefile->ExecutionStatusStack.push_back(&status);
+}
+
+cmMakefile::CallRAII::~CallRAII()
+{
+  if (this->Makefile) {
+    this->Detach();
+  }
+}
+
+cmMakefile* cmMakefile::CallRAII::Detach()
+{
+  assert(this->Makefile);
+
+  this->Makefile->ExecutionStatusStack.pop_back();
+  --this->Makefile->RecursionDepth;
+  this->Makefile->Backtrace = this->Makefile->Backtrace.Pop();
+
+  auto* const mf = this->Makefile;
+  this->Makefile = nullptr;
+  return mf;
+}
+
 // Helper class to make sure the call stack is valid.
-class cmMakefileCall
+class cmMakefile::CallScope : public CallRAII
 {
 public:
-  cmMakefileCall(cmMakefile* mf, cmListFileFunction const& lff,
-                 cm::optional<std::string> deferId, cmExecutionStatus& status)
-    : Makefile(mf)
+  CallScope(cmMakefile* mf, cmListFileFunction const& lff,
+            cm::optional<std::string> deferId, cmExecutionStatus& status)
+    : CallScope{ mf, lff,
+                 cmListFileContext::FromListFileFunction(
+                   lff, mf->StateSnapshot.GetExecutionListFile(),
+                   std::move(deferId)),
+                 status }
+  {
+  }
+
+  CallScope(cmMakefile* mf, cmListFileFunction const& lff,
+            cmListFileContext const& lfc, cmExecutionStatus& status)
+    : CallRAII{ mf, lfc, status }
   {
-    cmListFileContext const& lfc = cmListFileContext::FromListFileFunction(
-      lff, this->Makefile->StateSnapshot.GetExecutionListFile(),
-      std::move(deferId));
-    this->Makefile->Backtrace = this->Makefile->Backtrace.Push(lfc);
-    ++this->Makefile->RecursionDepth;
-    this->Makefile->ExecutionStatusStack.push_back(&status);
 #if !defined(CMAKE_BOOTSTRAP)
     this->ProfilingDataRAII =
       this->Makefile->GetCMakeInstance()->CreateProfilingEntry(
@@ -440,28 +479,25 @@ public:
 #endif
   }
 
-  ~cmMakefileCall()
+  ~CallScope()
   {
 #if !defined(CMAKE_BOOTSTRAP)
     this->ProfilingDataRAII.reset();
 #endif
-    this->Makefile->ExecutionStatusStack.pop_back();
-    --this->Makefile->RecursionDepth;
-    this->Makefile->Backtrace = this->Makefile->Backtrace.Pop();
+    auto* const mf = this->Detach();
 #ifdef CMake_ENABLE_DEBUGGER
-    if (this->Makefile->GetCMakeInstance()->GetDebugAdapter()) {
-      this->Makefile->GetCMakeInstance()
-        ->GetDebugAdapter()
-        ->OnEndFunctionCall();
+    if (mf->GetCMakeInstance()->GetDebugAdapter()) {
+      mf->GetCMakeInstance()->GetDebugAdapter()->OnEndFunctionCall();
     }
+#else
+    static_cast<void>(mf);
 #endif
   }
 
-  cmMakefileCall(const cmMakefileCall&) = delete;
-  cmMakefileCall& operator=(const cmMakefileCall&) = delete;
+  CallScope(const CallScope&) = delete;
+  CallScope& operator=(const CallScope&) = delete;
 
 private:
-  cmMakefile* Makefile;
 #if !defined(CMAKE_BOOTSTRAP)
   cm::optional<cmMakefileProfilingData::RAII> ProfilingDataRAII;
 #endif
@@ -485,7 +521,7 @@ bool cmMakefile::ExecuteCommand(const cmListFileFunction& lff,
   }
 
   // Place this call on the call stack.
-  cmMakefileCall stack_manager(this, lff, std::move(deferId), status);
+  CallScope stack_manager(this, lff, std::move(deferId), status);
   static_cast<void>(stack_manager);
 
   // Check for maximum recursion depth.

Source/cmMakefile.h

@@ -1054,6 +1054,9 @@ public:
   public:
     FindPackageStackRAII(cmMakefile* mf, std::string const& pkg);
     ~FindPackageStackRAII();
+
+    FindPackageStackRAII(FindPackageStackRAII const&) = delete;
+    FindPackageStackRAII& operator=(FindPackageStackRAII const&) = delete;
   };
 
   class DebugFindPkgRAII
@@ -1064,6 +1067,28 @@ public:
   public:
     DebugFindPkgRAII(cmMakefile* mf, std::string const& pkg);
     ~DebugFindPkgRAII();
+
+    DebugFindPkgRAII(DebugFindPkgRAII const&) = delete;
+    DebugFindPkgRAII& operator=(DebugFindPkgRAII const&) = delete;
+  };
+
+  class CallRAII
+  {
+  public:
+    CallRAII(cmMakefile* mf, std::string const& file,
+             cmExecutionStatus& status);
+    ~CallRAII();
+
+    CallRAII(CallRAII const&) = delete;
+    CallRAII& operator=(CallRAII const&) = delete;
+
+  protected:
+    CallRAII(cmMakefile* mf, cmListFileContext const& lfc,
+             cmExecutionStatus& status);
+
+    cmMakefile* Detach();
+
+    cmMakefile* Makefile;
   };
 
   bool GetDebugFindPkgMode() const;
@@ -1195,8 +1220,10 @@ private:
   std::vector<std::unique_ptr<cmGeneratorExpressionEvaluationFile>>
     EvaluationFiles;
 
+  class CallScope;
+  friend class CallScope;
+
   std::vector<cmExecutionStatus*> ExecutionStatusStack;
-  friend class cmMakefileCall;
   friend class cmParseFileScope;
 
   std::vector<std::unique_ptr<cmTarget>> ImportedTargetsOwned;

Source/cmMessenger.cxx

@@ -116,6 +116,7 @@ void PrintCallStack(std::ostream& out, cmListFileBacktrace bt,
   if (bt.Empty()) {
     return;
   }
+  std::string lastFilePath = bt.Top().FilePath;
   bt = bt.Pop();
   if (bt.Empty()) {
     return;
@@ -125,15 +126,19 @@ void PrintCallStack(std::ostream& out, cmListFileBacktrace bt,
   for (; !bt.Empty(); bt = bt.Pop()) {
     cmListFileContext lfc = bt.Top();
     if (lfc.Name.empty() &&
-        lfc.Line != cmListFileContext::DeferPlaceholderLine) {
-      // Skip this whole-file scope.  When we get here we already will
-      // have printed a more-specific context within the file.
+        lfc.Line != cmListFileContext::DeferPlaceholderLine &&
+        lfc.FilePath == lastFilePath) {
+      // An entry with no function name is frequently preceded (in the stack)
+      // by a more specific entry.  When this happens (as verified by the
+      // preceding entry referencing the same file path), skip the less
+      // specific entry, as we have already printed the more specific one.
       continue;
     }
     if (first) {
       first = false;
       out << "Call Stack (most recent call first):\n";
     }
+    lastFilePath = lfc.FilePath;
     if (topSource) {
       lfc.FilePath = cmSystemTools::RelativeIfUnder(*topSource, lfc.FilePath);
     }