Merge topic 'simplify-execution-context'

07c1bdda3d cmMakefile: Replace GetExecutionFilePath with the top of the Backtrace
727ed0c403 cmMakefile: Simplify ExpandArguments signature
e456dae669 cmConditionEvaluator: Remove extra copy of execution context
0100a4943e cmMakefile: Remove now-unused overload of GetBacktrace
dc49abcb89 if,while: Clarify condition backtrace construction
68af831505 cmMakefile: Inline GetExecutionContext at call sites
280f3918f3 cmMakefile: Simplify GetExecutionContext implementation
0e59b45dfc cmListFileCache: Add explicit constructors

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

Source/cmCMakeLanguageCommand.cxx

@@ -39,7 +39,7 @@ bool cmCMakeLanguageCommand(std::vector<cmListFileArgument> const& args,
   }
 
   cmMakefile& makefile = status.GetMakefile();
-  cmListFileContext context = makefile.GetExecutionContext();
+  cmListFileContext context = makefile.GetBacktrace().Top();
 
   bool result = false;
 

Source/cmConditionEvaluator.cxx

@@ -56,10 +56,8 @@ static std::string const keyVERSION_LESS = "VERSION_LESS";
 static std::string const keyVERSION_LESS_EQUAL = "VERSION_LESS_EQUAL";
 
 cmConditionEvaluator::cmConditionEvaluator(cmMakefile& makefile,
-                                           cmListFileContext context,
                                            cmListFileBacktrace bt)
   : Makefile(makefile)
-  , ExecutionContext(std::move(context))
   , Backtrace(std::move(bt))
   , Policy12Status(makefile.GetPolicyStatus(cmPolicies::CMP0012))
   , Policy54Status(makefile.GetPolicyStatus(cmPolicies::CMP0054))
@@ -147,8 +145,7 @@ cmProp cmConditionEvaluator::GetDefinitionIfUnquoted(
 
   if (def && argument.WasQuoted() &&
       this->Policy54Status == cmPolicies::WARN) {
-    if (!this->Makefile.HasCMP0054AlreadyBeenReported(
-          this->ExecutionContext)) {
+    if (!this->Makefile.HasCMP0054AlreadyBeenReported(this->Backtrace.Top())) {
       std::ostringstream e;
       e << (cmPolicies::GetPolicyWarning(cmPolicies::CMP0054)) << "\n";
       e << "Quoted variables like \"" << argument.GetValue()
@@ -191,8 +188,7 @@ bool cmConditionEvaluator::IsKeyword(std::string const& keyword,
 
   if (isKeyword && argument.WasQuoted() &&
       this->Policy54Status == cmPolicies::WARN) {
-    if (!this->Makefile.HasCMP0054AlreadyBeenReported(
-          this->ExecutionContext)) {
+    if (!this->Makefile.HasCMP0054AlreadyBeenReported(this->Backtrace.Top())) {
       std::ostringstream e;
       e << cmPolicies::GetPolicyWarning(cmPolicies::CMP0054) << "\n";
       e << "Quoted keywords like \"" << argument.GetValue()

Source/cmConditionEvaluator.h

@@ -21,8 +21,7 @@ class cmConditionEvaluator
 public:
   using cmArgumentList = std::list<cmExpandedCommandArgument>;
 
-  cmConditionEvaluator(cmMakefile& makefile, cmListFileContext context,
-                       cmListFileBacktrace bt);
+  cmConditionEvaluator(cmMakefile& makefile, cmListFileBacktrace bt);
 
   // this is a shared function for both If and Else to determine if the
   // arguments were valid, and if so, was the response true. If there is
@@ -79,7 +78,6 @@ private:
                     MessageType& status);
 
   cmMakefile& Makefile;
-  cmListFileContext ExecutionContext;
   cmListFileBacktrace Backtrace;
   cmPolicies::PolicyStatus Policy12Status;
   cmPolicies::PolicyStatus Policy54Status;

Source/cmFunctionCommand.cxx

@@ -147,8 +147,7 @@ bool cmFunctionFunctionBlocker::ArgumentsMatch(cmListFileFunction const& lff,
                                                cmMakefile& mf) const
 {
   std::vector<std::string> expandedArguments;
-  mf.ExpandArguments(lff.Arguments, expandedArguments,
-                     this->GetStartingContext().FilePath.c_str());
+  mf.ExpandArguments(lff.Arguments, expandedArguments);
   return expandedArguments.empty() ||
     expandedArguments.front() == this->Args.front();
 }

Source/cmIfCommand.cxx

@@ -75,10 +75,12 @@ bool cmIfFunctionBlocker::Replay(std::vector<cmListFileFunction> functions,
     if (scopeDepth == 0 && func.Name.Lower == "else") {
 
       if (this->ElseSeen) {
-        cmListFileBacktrace bt = mf.GetBacktrace(func);
+        cmListFileBacktrace elseBT = mf.GetBacktrace().Push(
+          cmListFileContext{ func.Name.Original,
+                             this->GetStartingContext().FilePath, func.Line });
         mf.GetCMakeInstance()->IssueMessage(
           MessageType::FATAL_ERROR,
-          "A duplicate ELSE command was found inside an IF block.", bt);
+          "A duplicate ELSE command was found inside an IF block.", elseBT);
         cmSystemTools::SetFatalErrorOccured();
         return true;
       }
@@ -93,11 +95,12 @@ bool cmIfFunctionBlocker::Replay(std::vector<cmListFileFunction> functions,
         mf.PrintCommandTrace(func);
       }
     } else if (scopeDepth == 0 && func.Name.Lower == "elseif") {
+      cmListFileBacktrace elseifBT = mf.GetBacktrace().Push(cmListFileContext{
+        func.Name.Original, this->GetStartingContext().FilePath, func.Line });
       if (this->ElseSeen) {
-        cmListFileBacktrace bt = mf.GetBacktrace(func);
         mf.GetCMakeInstance()->IssueMessage(
           MessageType::FATAL_ERROR,
-          "An ELSEIF command was found after an ELSE command.", bt);
+          "An ELSEIF command was found after an ELSE command.", elseifBT);
         cmSystemTools::SetFatalErrorOccured();
         return true;
       }
@@ -117,12 +120,7 @@ bool cmIfFunctionBlocker::Replay(std::vector<cmListFileFunction> functions,
 
         MessageType messType;
 
-        cmListFileContext conditionContext =
-          cmListFileContext::FromCommandContext(
-            func, this->GetStartingContext().FilePath);
-
-        cmConditionEvaluator conditionEvaluator(mf, conditionContext,
-                                                mf.GetBacktrace(func));
+        cmConditionEvaluator conditionEvaluator(mf, elseifBT);
 
         bool isTrue =
           conditionEvaluator.IsTrue(expandedArguments, errorString, messType);
@@ -130,8 +128,7 @@ bool cmIfFunctionBlocker::Replay(std::vector<cmListFileFunction> functions,
         if (!errorString.empty()) {
           std::string err =
             cmStrCat(cmIfCommandError(expandedArguments), errorString);
-          cmListFileBacktrace bt = mf.GetBacktrace(func);
-          mf.GetCMakeInstance()->IssueMessage(messType, err, bt);
+          mf.GetCMakeInstance()->IssueMessage(messType, err, elseifBT);
           if (messType == MessageType::FATAL_ERROR) {
             cmSystemTools::SetFatalErrorOccured();
             return true;
@@ -178,8 +175,7 @@ bool cmIfCommand(std::vector<cmListFileArgument> const& args,
 
   MessageType status;
 
-  cmConditionEvaluator conditionEvaluator(
-    makefile, makefile.GetExecutionContext(), makefile.GetBacktrace());
+  cmConditionEvaluator conditionEvaluator(makefile, makefile.GetBacktrace());
 
   bool isTrue =
     conditionEvaluator.IsTrue(expandedArguments, errorString, status);

Source/cmListFileCache.h

@@ -73,6 +73,14 @@ public:
   std::string FilePath;
   long Line = 0;
 
+  cmListFileContext() = default;
+  cmListFileContext(std::string name, std::string filePath, long line)
+    : Name(std::move(name))
+    , FilePath(std::move(filePath))
+    , Line(line)
+  {
+  }
+
   static cmListFileContext FromCommandContext(cmCommandContext const& lfcc,
                                               std::string const& fileName)
   {

Source/cmMacroCommand.cxx

@@ -157,8 +157,7 @@ bool cmMacroFunctionBlocker::ArgumentsMatch(cmListFileFunction const& lff,
                                             cmMakefile& mf) const
 {
   std::vector<std::string> expandedArguments;
-  mf.ExpandArguments(lff.Arguments, expandedArguments,
-                     this->GetStartingContext().FilePath.c_str());
+  mf.ExpandArguments(lff.Arguments, expandedArguments);
   return expandedArguments.empty() || expandedArguments[0] == this->Args[0];
 }
 

Source/cmMakefile.cxx

@@ -274,31 +274,12 @@ cmListFileBacktrace cmMakefile::GetBacktrace() const
   return this->Backtrace;
 }
 
-cmListFileBacktrace cmMakefile::GetBacktrace(cmCommandContext const& cc) const
-{
-  cmListFileContext lfc;
-  lfc.Name = cc.Name.Original;
-  lfc.Line = cc.Line;
-  lfc.FilePath = this->StateSnapshot.GetExecutionListFile();
-  return this->Backtrace.Push(lfc);
-}
-
-cmListFileContext cmMakefile::GetExecutionContext() const
-{
-  cmListFileContext const& cur = this->Backtrace.Top();
-  cmListFileContext lfc;
-  lfc.Name = cur.Name;
-  lfc.Line = cur.Line;
-  lfc.FilePath = this->StateSnapshot.GetExecutionListFile();
-  return lfc;
-}
-
 void cmMakefile::PrintCommandTrace(const cmListFileFunction& lff) const
 {
   // Check if current file in the list of requested to trace...
   std::vector<std::string> const& trace_only_this_files =
     this->GetCMakeInstance()->GetTraceSources();
-  std::string const& full_path = this->GetExecutionFilePath();
+  std::string const& full_path = this->GetBacktrace().Top().FilePath;
   std::string const& only_filename = cmSystemTools::GetFilenameName(full_path);
   bool trace = trace_only_this_files.empty();
   if (!trace) {
@@ -597,7 +578,7 @@ void cmMakefile::IncludeScope::EnforceCMP0011()
         std::ostringstream w;
         w << cmPolicies::GetPolicyWarning(cmPolicies::CMP0011) << "\n"
           << "The included script\n  "
-          << this->Makefile->GetExecutionFilePath() << "\n"
+          << this->Makefile->GetBacktrace().Top().FilePath << "\n"
           << "affects policy settings.  "
           << "CMake is implying the NO_POLICY_SCOPE option for compatibility, "
           << "so the effects are applied to the including context.";
@@ -610,7 +591,7 @@ void cmMakefile::IncludeScope::EnforceCMP0011()
       /* clang-format off */
       e << cmPolicies::GetRequiredPolicyError(cmPolicies::CMP0011) << "\n"
         << "The included script\n  "
-        << this->Makefile->GetExecutionFilePath() << "\n"
+        << this->Makefile->GetBacktrace().Top().FilePath << "\n"
         << "affects policy settings, so it requires this policy to be set.";
       /* clang-format on */
       this->Makefile->IssueMessage(MessageType::FATAL_ERROR, e.str());
@@ -3347,20 +3328,10 @@ bool cmMakefile::IsLoopBlock() const
   return !this->LoopBlockCounter.empty() && this->LoopBlockCounter.top() > 0;
 }
 
-std::string const& cmMakefile::GetExecutionFilePath() const
-{
-  assert(this->StateSnapshot.IsValid());
-  return this->StateSnapshot.GetExecutionListFile();
-}
-
 bool cmMakefile::ExpandArguments(std::vector<cmListFileArgument> const& inArgs,
-                                 std::vector<std::string>& outArgs,
-                                 const char* filename) const
+                                 std::vector<std::string>& outArgs) const
 {
-  if (!filename) {
-    auto const& efp = this->GetExecutionFilePath();
-    filename = efp.c_str();
-  }
+  std::string const& filename = this->GetBacktrace().Top().FilePath;
   std::string value;
   outArgs.reserve(inArgs.size());
   for (cmListFileArgument const& i : inArgs) {
@@ -3371,8 +3342,8 @@ bool cmMakefile::ExpandArguments(std::vector<cmListFileArgument> const& inArgs,
     }
     // Expand the variables in the argument.
     value = i.Value;
-    this->ExpandVariablesInString(value, false, false, false, filename, i.Line,
-                                  false, false);
+    this->ExpandVariablesInString(value, false, false, false, filename.c_str(),
+                                  i.Line, false, false);
 
     // If the argument is quoted, it should be one argument.
     // Otherwise, it may be a list of arguments.
@@ -3387,12 +3358,9 @@ bool cmMakefile::ExpandArguments(std::vector<cmListFileArgument> const& inArgs,
 
 bool cmMakefile::ExpandArguments(
   std::vector<cmListFileArgument> const& inArgs,
-  std::vector<cmExpandedCommandArgument>& outArgs, const char* filename) const
+  std::vector<cmExpandedCommandArgument>& outArgs) const
 {
-  if (!filename) {
-    auto const& efp = this->GetExecutionFilePath();
-    filename = efp.c_str();
-  }
+  std::string const& filename = this->GetBacktrace().Top().FilePath;
   std::string value;
   outArgs.reserve(inArgs.size());
   for (cmListFileArgument const& i : inArgs) {
@@ -3403,8 +3371,8 @@ bool cmMakefile::ExpandArguments(
     }
     // Expand the variables in the argument.
     value = i.Value;
-    this->ExpandVariablesInString(value, false, false, false, filename, i.Line,
-                                  false, false);
+    this->ExpandVariablesInString(value, false, false, false, filename.c_str(),
+                                  i.Line, false, false);
 
     // If the argument is quoted, it should be one argument.
     // Otherwise, it may be a list of arguments.
@@ -3424,7 +3392,7 @@ void cmMakefile::AddFunctionBlocker(std::unique_ptr<cmFunctionBlocker> fb)
 {
   if (!this->ExecutionStatusStack.empty()) {
     // Record the context in which the blocker is created.
-    fb->SetStartingContext(this->GetExecutionContext());
+    fb->SetStartingContext(this->Backtrace.Top());
   }
 
   this->FunctionBlockers.push(std::move(fb));

Source/cmMakefile.h

@@ -640,8 +640,6 @@ public:
    * Get the current context backtrace.
    */
   cmListFileBacktrace GetBacktrace() const;
-  cmListFileBacktrace GetBacktrace(cmCommandContext const& lfc) const;
-  cmListFileContext GetExecutionContext() const;
 
   /**
    * Get the vector of  files created by this makefile
@@ -734,12 +732,9 @@ public:
    * variable replacement and list expansion.
    */
   bool ExpandArguments(std::vector<cmListFileArgument> const& inArgs,
-                       std::vector<std::string>& outArgs,
-                       const char* filename = nullptr) const;
-
+                       std::vector<std::string>& outArgs) const;
   bool ExpandArguments(std::vector<cmListFileArgument> const& inArgs,
-                       std::vector<cmExpandedCommandArgument>& outArgs,
-                       const char* filename = nullptr) const;
+                       std::vector<cmExpandedCommandArgument>& outArgs) const;
 
   /**
    * Get the instance
@@ -942,8 +937,6 @@ public:
 
   const char* GetDefineFlagsCMP0059() const;
 
-  std::string const& GetExecutionFilePath() const;
-
   void EnforceDirectoryLevelRules() const;
 
   void AddEvaluationFile(

Source/cmTargetLinkLibrariesCommand.cxx

@@ -10,6 +10,7 @@
 #include "cmExecutionStatus.h"
 #include "cmGeneratorExpression.h"
 #include "cmGlobalGenerator.h"
+#include "cmListFileCache.h"
 #include "cmMakefile.h"
 #include "cmMessageType.h"
 #include "cmPolicies.h"
@@ -386,7 +387,7 @@ bool TLL::HandleLibrary(ProcessingState currentProcessingState,
     ? cmTarget::KeywordTLLSignature
     : cmTarget::PlainTLLSignature;
   if (!this->Target->PushTLLCommandTrace(
-        sig, this->Makefile.GetExecutionContext())) {
+        sig, this->Makefile.GetBacktrace().Top())) {
     std::ostringstream e;
     const char* modal = nullptr;
     MessageType messageType = MessageType::AUTHOR_WARNING;

Source/cmWhileCommand.cxx

@@ -17,6 +17,7 @@
 #include "cmMakefile.h"
 #include "cmMessageType.h"
 #include "cmSystemTools.h"
+#include "cmake.h"
 
 class cmWhileFunctionBlocker : public cmFunctionBlocker
 {
@@ -66,14 +67,9 @@ bool cmWhileFunctionBlocker::Replay(std::vector<cmListFileFunction> functions,
   mf.ExpandArguments(this->Args, expandedArguments);
   MessageType messageType;
 
-  cmListFileContext execContext = this->GetStartingContext();
-
-  cmCommandContext commandContext;
-  commandContext.Line = execContext.Line;
-  commandContext.Name = execContext.Name;
-
-  cmConditionEvaluator conditionEvaluator(mf, this->GetStartingContext(),
-                                          mf.GetBacktrace(commandContext));
+  cmListFileBacktrace whileBT =
+    mf.GetBacktrace().Push(this->GetStartingContext());
+  cmConditionEvaluator conditionEvaluator(mf, whileBT);
 
   bool isTrue =
     conditionEvaluator.IsTrue(expandedArguments, errorString, messageType);
@@ -90,7 +86,7 @@ bool cmWhileFunctionBlocker::Replay(std::vector<cmListFileFunction> functions,
       err += "(";
       err += errorString;
       err += ").";
-      mf.IssueMessage(messageType, err);
+      mf.GetCMakeInstance()->IssueMessage(messageType, err, whileBT);
       if (messageType == MessageType::FATAL_ERROR) {
         cmSystemTools::SetFatalErrorOccured();
         return true;