[flang] Make messages more like clang's.

Original-commit: flang-compiler/f18@176cdf8e6ce79bb6a8fb2d947b218d17f315b071
Reviewed-on: https://github.com/flang-compiler/f18/pull/81
Tree-same-pre-rewrite: false

diff --git a/flang/lib/parser/debug-parser.cc b/flang/lib/parser/debug-parser.cc
index 72f1a6f..eaaa48b 100644 (file)
--- a/flang/lib/parser/debug-parser.cc
+++ b/flang/lib/parser/debug-parser.cc
@@ -22,13 +22,13 @@ namespace Fortran::parser {
 std::optional<Success> DebugParser::Parse(ParseState &state) const {
   if (auto ustate = state.userState()) {
     if (auto out = ustate->debugOutput()) {
-      const CookedSource &cooked{ustate->cooked()};
-      if (auto context = state.context()) {
-        context->Emit(*out, cooked);
+      std::string note{str_, length_};
+      Message message{state.GetLocation(),
+          MessageFormattedText{"parser debug: %s"_en_US, note.data()}};
+      if (Message * context{state.context().get()}) {
+        message.set_context(context);
       }
-      Provenance p{cooked.GetProvenance(state.GetLocation()).start()};
-      cooked.allSources().Identify(*out, p, "", true);
-      *out << "   parser debug: " << std::string{str_, length_} << "\n\n";
+      message.Emit(*out, ustate->cooked(), true);
     }
   }
   return {Success{}};

diff --git a/flang/lib/parser/message.cc b/flang/lib/parser/message.cc
index abaf4f7..47ba916 100644 (file)
--- a/flang/lib/parser/message.cc
+++ b/flang/lib/parser/message.cc
@@ -14,11 +14,13 @@
 
 #include "message.h"
 #include "char-set.h"
+#include <algorithm>
 #include <cstdarg>
 #include <cstddef>
 #include <cstdio>
 #include <cstring>
 #include <string>
+#include <vector>
 
 namespace Fortran::parser {
 
@@ -112,20 +114,21 @@ ProvenanceRange Message::GetProvenance(const CookedSource &cooked) const {
 void Message::Emit(
     std::ostream &o, const CookedSource &cooked, bool echoSourceLine) const {
   ProvenanceRange provenanceRange{GetProvenance(cooked)};
-  bool doIdentify{true};
-  if (context_) {
-    bool sameProvenance{provenanceRange == context_->GetProvenance(cooked)};
-    context_->Emit(o, cooked, echoSourceLine && sameProvenance);
-    doIdentify = !sameProvenance;
-  }
-  if (doIdentify) {
-    cooked.allSources().Identify(o, provenanceRange, "", echoSourceLine);
-  }
-  o << "   ";
+  std::string text;
   if (isFatal_) {
-    o << "ERROR: ";
+    text += "ERROR: ";
+  }
+  text += ToString();
+  cooked.allSources().EmitMessage(o, provenanceRange, text, echoSourceLine);
+  for (const Message *context{context_.get()}; context != nullptr;
+       context = context->context_.get()) {
+    ProvenanceRange contextProvenance{context->GetProvenance(cooked)};
+    text = "in the context: ";
+    text += context->ToString();
+    cooked.allSources().EmitMessage(o, contextProvenance, text,
+        echoSourceLine && contextProvenance != provenanceRange);
+    provenanceRange = contextProvenance;
   }
-  o << ToString() << '\n';
 }
 
 void Messages::Incorporate(Messages &that) {
@@ -143,16 +146,16 @@ void Messages::Copy(const Messages &that) {
   }
 }
 
-void Messages::Emit(std::ostream &o, const CookedSource &cooked,
-    const char *prefix, bool echoSourceLines) const {
+void Messages::Emit(
+    std::ostream &o, const CookedSource &cooked, bool echoSourceLines) const {
+  std::vector<const Message *> sorted;
   for (const auto &msg : messages_) {
-    if (prefix) {
-      o << prefix;
-    }
-    if (msg.context()) {
-      o << "In the context ";
-    }
-    msg.Emit(o, cooked, echoSourceLines);
+    sorted.push_back(&msg);
+  }
+  std::sort(sorted.begin(), sorted.end(),
+      [](const Message *x, const Message *y) { return *x < *y; });
+  for (const Message *msg : sorted) {
+    msg->Emit(o, cooked, echoSourceLines);
   }
 }
 

diff --git a/flang/lib/parser/message.h b/flang/lib/parser/message.h
index 04ba03d..babf1fb 100644 (file)
--- a/flang/lib/parser/message.h
+++ b/flang/lib/parser/message.h
@@ -207,9 +207,8 @@ public:
 
   void Incorporate(Messages &);
   void Copy(const Messages &);
-
   void Emit(std::ostream &, const CookedSource &cooked,
-      const char *prefix = nullptr, bool echoSourceLines = true) const;
+      bool echoSourceLines = true) const;
 
   bool AnyFatalError() const;
 

diff --git a/flang/lib/parser/parsing.cc b/flang/lib/parser/parsing.cc
index 9b69531..c4aefc6 100644 (file)
--- a/flang/lib/parser/parsing.cc
+++ b/flang/lib/parser/parsing.cc
@@ -122,8 +122,7 @@ bool Parsing::ForTesting(std::string path, std::ostream &err) {
   Parse();
   messages_.Emit(err, cooked_);
   if (!consumedWholeFile_) {
-    err << "f18 parser FAIL; final position: ";
-    Identify(err, finalRestingPlace_, "   ");
+    EmitMessage(err, finalRestingPlace_, "parser FAIL; final position");
     return false;
   }
   if (messages_.AnyFatalError() || !parseTree_.has_value()) {

diff --git a/flang/lib/parser/parsing.h b/flang/lib/parser/parsing.h
index 875411e..dbb1a3d 100644 (file)
--- a/flang/lib/parser/parsing.h
+++ b/flang/lib/parser/parsing.h
@@ -61,10 +61,10 @@ public:
   void Parse(std::ostream *debugOutput = nullptr);
   void ClearLog();
 
-  void Identify(std::ostream &o, const char *at, const std::string &prefix,
+  void EmitMessage(std::ostream &o, const char *at, const std::string &message,
       bool echoSourceLine = false) const {
-    allSources_.Identify(
-        o, cooked_.GetProvenance(at).start(), prefix, echoSourceLine);
+    allSources_.EmitMessage(
+        o, cooked_.GetProvenance(at).start(), message, echoSourceLine);
   }
 
   bool ForTesting(std::string path, std::ostream &);

diff --git a/flang/lib/parser/provenance.cc b/flang/lib/parser/provenance.cc
index c6e693e..fe39794 100644 (file)
--- a/flang/lib/parser/provenance.cc
+++ b/flang/lib/parser/provenance.cc
@@ -75,7 +75,7 @@ void OffsetToProvenanceMappings::RemoveLastBytes(std::size_t bytes) {
 }
 
 AllSources::AllSources() : range_{1, 1} {
-  // Start the origin_ array with a dummy that has a forced provenance,
+  // Start the origin_ array with a dummy entry that has a forced provenance,
   // so that provenance offset 0 remains reserved as an uninitialized
   // value.
   origin_.emplace_back(range_, std::string{'?'});
@@ -141,25 +141,26 @@ ProvenanceRange AllSources::AddCompilerInsertion(std::string text) {
   return covers;
 }
 
-void AllSources::Identify(std::ostream &o, ProvenanceRange range,
-    const std::string &prefix, bool echoSourceLine) const {
+void AllSources::EmitMessage(std::ostream &o, ProvenanceRange range,
+    const std::string &message, bool echoSourceLine) const {
   CHECK(IsValid(range));
-  static const std::string indented{prefix + "  "};
   const Origin &origin{MapToOrigin(range.start())};
   std::visit(
       visitors{
           [&](const Inclusion &inc) {
+            o << inc.source.path();
             std::size_t offset{origin.covers.MemberOffset(range.start())};
             std::pair<int, int> pos{inc.source.FindOffsetLineAndColumn(offset)};
-            o << prefix << "at line " << pos.first << ", column " << pos.second;
+            o << ':' << pos.first << ':' << pos.second;
+            o << ": " << message << '\n';
             if (echoSourceLine) {
-              o << ":\n" << indented << "  ";
               const char *text{inc.source.content() +
                   inc.source.GetLineStartOffset(pos.first)};
+              o << "  ";
               for (const char *p{text}; *p != '\n'; ++p) {
                 o << *p;
               }
-              o << '\n' << indented << "  ";
+              o << "\n  ";
               for (int j{1}; j < pos.second; ++j) {
                 char ch{text[j - 1]};
                 o << (ch == '\t' ? '\t' : ' ');
@@ -177,28 +178,20 @@ void AllSources::Identify(std::ostream &o, ProvenanceRange range,
                   }
                 }
               }
-              o << '\n' << prefix;
-            } else {
-              o << ' ';
+              o << '\n';
             }
-            o << "in the " << (inc.isModule ? "module " : "file ")
-              << inc.source.path();
             if (IsValid(origin.replaces)) {
-              o << (inc.isModule ? " used\n" : " included\n");
-              Identify(o, origin.replaces.start(), indented);
-            } else {
-              o << '\n';
+              EmitMessage(o, origin.replaces,
+                  inc.isModule ? "used here"s : "included here"s,
+                  echoSourceLine);
             }
           },
           [&](const Macro &mac) {
-            o << prefix << "in the expansion of a macro that was defined\n";
-            Identify(o, mac.definition.start(), indented, echoSourceLine);
-            o << prefix << "and called\n";
-            Identify(o, origin.replaces.start(), indented, echoSourceLine);
+            EmitMessage(o, origin.replaces, message, echoSourceLine);
+            EmitMessage(
+                o, mac.definition, "in a macro defined here", echoSourceLine);
             if (echoSourceLine) {
-              o << prefix << "and expanded to\n"
-                << indented << "  " << mac.expansion << '\n'
-                << indented << "  ";
+              o << "that expanded to:\n  " << mac.expansion << "\n  ";
               for (std::size_t j{0};
                    origin.covers.OffsetMember(j) < range.start(); ++j) {
                 o << (mac.expansion[j] == '\t' ? '\t' : ' ');
@@ -206,9 +199,7 @@ void AllSources::Identify(std::ostream &o, ProvenanceRange range,
               o << "^\n";
             }
           },
-          [&](const CompilerInsertion &ins) {
-            o << prefix << ins.text << '\n';
-          }},
+          [&](const CompilerInsertion &ins) { o << message << '\n'; }},
       origin.u);
 }
 
@@ -360,6 +351,10 @@ void AllSources::Dump(std::ostream &o) const {
                      }
                    }},
         m.u);
+    if (IsValid(m.replaces)) {
+      o << " replaces ";
+      DumpRange(o, m.replaces);
+    }
     o << '\n';
   }
 }

diff --git a/flang/lib/parser/provenance.h b/flang/lib/parser/provenance.h
index 709508c..48fbbf4 100644 (file)
--- a/flang/lib/parser/provenance.h
+++ b/flang/lib/parser/provenance.h
@@ -129,7 +129,7 @@ public:
   bool IsValid(ProvenanceRange range) const {
     return range.size() > 0 && range_.Contains(range);
   }
-  void Identify(std::ostream &, ProvenanceRange, const std::string &prefix,
+  void EmitMessage(std::ostream &, ProvenanceRange, const std::string &message,
       bool echoSourceLine = false) const;
   const SourceFile *GetSourceFile(
       Provenance, std::size_t *offset = nullptr) const;

diff --git a/flang/tools/f18/f18.cc b/flang/tools/f18/f18.cc
index 26321cd..7835db5 100644 (file)
--- a/flang/tools/f18/f18.cc
+++ b/flang/tools/f18/f18.cc
@@ -164,7 +164,7 @@ std::string CompileFortran(
   if (!parsing.messages().empty() &&
       (driver.warningsAreErrors || parsing.messages().AnyFatalError())) {
     std::cerr << driver.prefix << "could not scan " << path << '\n';
-    parsing.messages().Emit(std::cerr, parsing.cooked(), driver.prefix);
+    parsing.messages().Emit(std::cerr, parsing.cooked());
     exitStatus = EXIT_FAILURE;
     return {};
   }
@@ -182,10 +182,10 @@ std::string CompileFortran(
     return {};
   }
   parsing.ClearLog();
-  parsing.messages().Emit(std::cerr, parsing.cooked(), driver.prefix);
+  parsing.messages().Emit(std::cerr, parsing.cooked());
   if (!parsing.consumedWholeFile()) {
-    std::cerr << "f18 parser FAIL; final position: ";
-    parsing.Identify(std::cerr, parsing.finalRestingPlace(), "   ");
+    parsing.EmitMessage(std::cerr, parsing.finalRestingPlace(),
+                        "parser FAIL (final position)");
     exitStatus = EXIT_FAILURE;
     return {};
   }