cmListFile: Use cmMakefile to issue messages

Modify cmListFile[Parser] to use cmMakefile rather than cmMessenger as
the interface for issuing messages. This is necessary to provide a
context for diagnostic messages so that they can be controlled via the
diagnostic state (which is not owned by cmMessenger).

Note that we actually bypass the cmMakefile when issuing errors, as
these cannot be disabled (so the diagnostic context doesn't matter), and
we want the calling cmMakefile to still be able to issue its own errors,
which would be suppressed if the cmMakefile has already issued an error.

Source/cmListFileCache.cxx

@@ -15,10 +15,11 @@
 
 #include "cmList.h"
 #include "cmListFileLexer.h"
+#include "cmMakefile.h"
 #include "cmMessageType.h"
-#include "cmMessenger.h"
 #include "cmStringAlgorithms.h"
 #include "cmSystemTools.h"
+#include "cmake.h"
 
 namespace {
 
@@ -48,7 +49,7 @@ class cmListFileParser
 {
 public:
   cmListFileParser(cmListFile* lf, cmListFileBacktrace lfbt,
-                   cmMessenger* messenger, std::string const& filename);
+                   cmMakefile const* mf, std::string const& filename);
   cmListFileParser(cmListFileParser const&) = delete;
   cmListFileParser& operator=(cmListFileParser const&) = delete;
 
@@ -74,7 +75,7 @@ private:
 
   cmListFile* ListFile;
   cmListFileBacktrace Backtrace;
-  cmMessenger* Messenger;
+  cmMakefile const* Makefile;
   std::string const& FileName;
   std::unique_ptr<cmListFileLexer, void (*)(cmListFileLexer*)> Lexer;
   std::string FunctionName;
@@ -84,11 +85,11 @@ private:
 };
 
 cmListFileParser::cmListFileParser(cmListFile* lf, cmListFileBacktrace lfbt,
-                                   cmMessenger* messenger,
+                                   cmMakefile const* mf,
                                    std::string const& filename)
   : ListFile(lf)
   , Backtrace(std::move(lfbt))
-  , Messenger(messenger)
+  , Makefile(mf)
   , FileName(filename)
   , Lexer(cmListFileLexer_New(), cmListFileLexer_Delete)
 {
@@ -96,18 +97,23 @@ cmListFileParser::cmListFileParser(cmListFile* lf, cmListFileBacktrace lfbt,
 
 void cmListFileParser::IssueFileOpenError(std::string const& text) const
 {
-  this->Messenger->IssueMessage(MessageType::FATAL_ERROR, text,
-                                this->Backtrace);
+  if (this->Makefile) {
+    this->Makefile->GetCMakeInstance()->IssueMessage(MessageType::FATAL_ERROR,
+                                                     text, this->Backtrace);
+  }
 }
 
 void cmListFileParser::IssueError(std::string const& text) const
 {
-  cmListFileContext lfc;
-  lfc.FilePath = this->FileName;
-  lfc.Line = cmListFileLexer_GetCurrentLine(this->Lexer.get());
-  cmListFileBacktrace lfbt = this->Backtrace;
-  lfbt = lfbt.Push(lfc);
-  this->Messenger->IssueMessage(MessageType::FATAL_ERROR, text, lfbt);
+  if (this->Makefile) {
+    cmListFileContext lfc;
+    lfc.FilePath = this->FileName;
+    lfc.Line = cmListFileLexer_GetCurrentLine(this->Lexer.get());
+    cmListFileBacktrace lfbt = this->Backtrace;
+    lfbt = lfbt.Push(lfc);
+    this->Makefile->GetCMakeInstance()->IssueMessage(MessageType::FATAL_ERROR,
+                                                     text, lfbt);
+  }
   cmSystemTools::SetFatalErrorOccurred();
 }
 
@@ -194,10 +200,12 @@ bool cmListFileParser::Parse()
 
   // Check if all functions are nested properly.
   if (auto badNesting = this->CheckNesting()) {
-    this->Messenger->IssueMessage(
-      MessageType::FATAL_ERROR,
-      "Flow control statements are not properly nested.",
-      this->Backtrace.Push(*badNesting));
+    if (this->Makefile) {
+      this->Makefile->GetCMakeInstance()->IssueMessage(
+        MessageType::FATAL_ERROR,
+        "Flow control statements are not properly nested.",
+        this->Backtrace.Push(*badNesting));
+    }
     cmSystemTools::SetFatalErrorOccurred();
     return false;
   }
@@ -286,16 +294,18 @@ bool cmListFileParser::ParseFunction(cm::string_view name, long line)
     }
   }
 
-  cmListFileContext lfc;
-  lfc.FilePath = this->FileName;
-  lfc.Line = line;
-  cmListFileBacktrace lfbt = this->Backtrace;
-  lfbt = lfbt.Push(lfc);
-  this->Messenger->IssueMessage(
-    MessageType::FATAL_ERROR,
-    "Parse error.  Function missing ending \")\".  "
-    "End of file reached.",
-    lfbt);
+  if (this->Makefile) {
+    cmListFileContext lfc;
+    lfc.FilePath = this->FileName;
+    lfc.Line = line;
+    cmListFileBacktrace lfbt = this->Backtrace;
+    lfbt = lfbt.Push(lfc);
+    this->Makefile->GetCMakeInstance()->IssueMessage(
+      MessageType::FATAL_ERROR,
+      "Parse error.  Function missing ending \")\".  "
+      "End of file reached.",
+      lfbt);
+  }
   return false;
 }
 
@@ -320,10 +330,15 @@ bool cmListFileParser::AddArgument(cmListFileLexer_Token* token,
              "\n"
              "Argument not separated from preceding token by whitespace.");
   if (isError) {
-    this->Messenger->IssueMessage(MessageType::FATAL_ERROR, msg, lfbt);
+    if (this->Makefile) {
+      this->Makefile->GetCMakeInstance()->IssueMessage(
+        MessageType::FATAL_ERROR, msg, lfbt);
+    }
     return false;
   }
-  this->Messenger->IssueMessage(MessageType::AUTHOR_WARNING, msg, lfbt);
+  if (this->Makefile) {
+    this->Makefile->IssueMessage(MessageType::AUTHOR_WARNING, msg, lfbt);
+  }
   return true;
 }
 
@@ -422,7 +437,7 @@ cm::optional<cmListFileContext> cmListFileParser::CheckNesting() const
 
 } // anonymous namespace
 
-bool cmListFile::ParseFile(std::string const& filename, cmMessenger* messenger,
+bool cmListFile::ParseFile(std::string const& filename, cmMakefile const* mf,
                            cmListFileBacktrace const& lfbt)
 {
   if (!cmSystemTools::FileExists(filename) ||
@@ -430,16 +445,16 @@ bool cmListFile::ParseFile(std::string const& filename, cmMessenger* messenger,
     return false;
   }
 
-  cmListFileParser parser(this, lfbt, messenger, filename);
+  cmListFileParser parser(this, lfbt, mf, filename);
   return parser.ParseFile();
 }
 
 bool cmListFile::ParseString(cm::string_view str,
                              std::string const& virtual_filename,
-                             cmMessenger* messenger,
+                             cmMakefile const* mf,
                              cmListFileBacktrace const& lfbt)
 {
-  cmListFileParser parser(this, lfbt, messenger, virtual_filename);
+  cmListFileParser parser(this, lfbt, mf, virtual_filename);
   return parser.ParseString(str);
 }
 

Source/cmListFileCache.h

@@ -24,7 +24,7 @@
  * cmake list files.
  */
 
-class cmMessenger;
+class cmMakefile;
 
 struct cmListFileArgument
 {
@@ -239,11 +239,11 @@ std::vector<BT<std::string>> cmExpandListWithBacktrace(
 
 struct cmListFile
 {
-  bool ParseFile(std::string const& path, cmMessenger* messenger,
+  bool ParseFile(std::string const& path, cmMakefile const* mf,
                  cmListFileBacktrace const& lfbt);
 
   bool ParseString(cm::string_view str, std::string const& virtual_filename,
-                   cmMessenger* messenger, cmListFileBacktrace const& lfbt);
+                   cmMakefile const* mf, cmListFileBacktrace const& lfbt);
 
   std::vector<cmListFileFunction> Functions;
 };

Source/cmMakefile.cxx

@@ -710,8 +710,7 @@ bool cmMakefile::ReadDependentFile(std::string const& filename,
 #endif
 
   cmListFile listFile;
-  if (!listFile.ParseFile(filenametoread, this->GetMessenger(),
-                          this->Backtrace)) {
+  if (!listFile.ParseFile(filenametoread, this, this->Backtrace)) {
 #ifdef CMake_ENABLE_DEBUGGER
     if (this->GetCMakeInstance()->GetDebugAdapter()) {
       this->GetCMakeInstance()->GetDebugAdapter()->OnEndFileParse();
@@ -833,8 +832,7 @@ bool cmMakefile::ReadListFile(std::string const& filename)
 #endif
 
   cmListFile listFile;
-  if (!listFile.ParseFile(filenametoread, this->GetMessenger(),
-                          this->Backtrace)) {
+  if (!listFile.ParseFile(filenametoread, this, this->Backtrace)) {
 #ifdef CMake_ENABLE_DEBUGGER
     if (this->GetCMakeInstance()->GetDebugAdapter()) {
       this->GetCMakeInstance()->GetDebugAdapter()->OnEndFileParse();
@@ -868,8 +866,7 @@ bool cmMakefile::ReadListFileAsString(std::string const& content,
   ListFileScope scope(this, filenametoread);
 
   cmListFile listFile;
-  if (!listFile.ParseString(content, virtualFileName, this->GetMessenger(),
-                            this->Backtrace)) {
+  if (!listFile.ParseString(content, virtualFileName, this, this->Backtrace)) {
     return false;
   }
 
@@ -1628,8 +1625,7 @@ void cmMakefile::Configure()
 #endif
 
   cmListFile listFile;
-  if (!listFile.ParseFile(currentStart, this->GetMessenger(),
-                          this->Backtrace)) {
+  if (!listFile.ParseFile(currentStart, this, this->Backtrace)) {
 #ifdef CMake_ENABLE_DEBUGGER
     if (this->GetCMakeInstance()->GetDebugAdapter()) {
       this->GetCMakeInstance()->GetDebugAdapter()->OnEndFileParse();

Tests/CMakeLib/testDebugger.h

@@ -8,7 +8,6 @@
 #include "cmDebuggerAdapter.h"
 #include "cmDebuggerProtocol.h"
 #include "cmListFileCache.h"
-#include "cmMessenger.h"
 #include <cmcppdap/include/dap/io.h>
 #include <cmcppdap/include/dap/session.h>
 #include <cmcppdap/include/dap/types.h>
@@ -75,10 +74,9 @@ public:
   std::vector<cmListFileFunction> CreateListFileFunctions(
     char const* str, std::string const& filename)
   {
-    cmMessenger messenger;
     cmListFileBacktrace backtrace;
     cmListFile listfile;
-    listfile.ParseString(str, filename, &messenger, backtrace);
+    listfile.ParseString(str, filename, nullptr, backtrace);
     return listfile.Functions;
   }
 };

Tests/Fuzzing/cmListFileParserFuzzer.cxx

@@ -20,7 +20,6 @@
 #include <unistd.h>
 
 #include "cmListFileCache.h"
-#include "cmMessenger.h"
 #include "cmSystemTools.h"
 
 static constexpr size_t kMaxInputSize = 256 * 1024;
@@ -59,13 +58,10 @@ extern "C" int LLVMFuzzerTestOneInput(uint8_t const* data, size_t size)
     fclose(fp);
   }
 
-  // Create a messenger for error handling
-  cmMessenger messenger;
-
   // Parse the file
   cmListFile listFile;
   cmListFileBacktrace backtrace;
-  if (listFile.ParseFile(testFile.c_str(), &messenger, backtrace)) {
+  if (listFile.ParseFile(testFile.c_str(), nullptr, backtrace)) {
     // Successfully parsed - examine results
     for (auto const& func : listFile.Functions) {
       (void)func.LowerCaseName();