[flang] Improve appearance of message attachments

[klausler]

Summary

Error messages can have a list of attachments; these are used to point
to related source locations, supply additional information, and to
encapsulate error messages that were *not* emitted in a given context
to explain why a warning was justified.

This patch adds a message severity ("Because") for that last case,
and extends to AttachTo() API to provide a means for overriding
the severity of an attached message.

Some existing message attachments had their severities adjusted,
now that we're printing them. And operator==() for Message was
cleaned up while debugging after I noticed that it was recursively
O(N**2) and subject to returning a false positive.

[klausler]

Replace a call to "common::visit()" with the "std::visit()" used in llvm.

flang/include/flang/Parser/message.h

 
 namespace Fortran::parser {
 
-// Use "..."_err_en_US, "..."_warn_en_US, and "..."_en_US literals to define
-// the static text and fatality of a message.
-enum class Severity { Error, Warning, Portability, None };
+// Use "..."_err_en_US, "..."_warn_en_US, "..."_port_en_US, and "..."_en_US
+// string literals to define the static text and fatality of a message.
+//
+// Error: fatal error that prevents code and module file generation
+// Warning: likely problem,
+// Portability: nonstandard or obsolete features
+// Because: for AttachTo(), explanatory attachment in support of another message
+// Context (internal): attachment from SetContext()
+// None: everything else, common for attachments with source locations
+enum class Severity { Error, Warning, Portability, Because, Context, None };
 
 class MessageFixedText {
 public:

 
   CharBlock text() const { return text_; }
   Severity severity() const { return severity_; }
+  MessageFixedText &set_severity(Severity severity) {
+    severity_ = severity;
+    return *this;
+  }
   bool isFatal() const { return severity_ == Severity::Error; }
 
 private:

   const std::string &string() const { return string_; }
   bool isFatal() const { return severity_ == Severity::Error; }
   Severity severity() const { return severity_; }
+  MessageFormattedText &set_severity(Severity severity) {
+    severity_ = severity;
+    return *this;
+  }
   std::string MoveString() { return std::move(string_); }
 
 private:

       : location_{r}, text_{MessageFormattedText{
                           t, std::forward<A>(x), std::forward<As>(xs)...}} {}
 
-  bool attachmentIsContext() const { return attachmentIsContext_; }
   Reference attachment() const { return attachment_; }
 
   void SetContext(Message *c) {

   bool SortBefore(const Message &that) const;
   bool IsFatal() const;
   Severity severity() const;
+  Message &set_severity(Severity);
   std::string ToString() const;
   std::optional<ProvenanceRange> GetProvenanceRange(
       const AllCookedSources &) const;

   void ResolveProvenances(const AllCookedSources &);
   void Emit(llvm::raw_ostream &, const AllCookedSources &,
       bool echoSourceLines = true) const;
-  void AttachTo(Message &);
+  void AttachTo(Message &, std::optional<Severity> = std::nullopt);
   bool AnyFatalError() const;
 
 private:

flang/lib/Parser/message.cpp

       text_);
 }
 
+Message &Message::set_severity(Severity severity) {
+  std::visit(
+      common::visitors{
+          [](const MessageExpectedText &) {},
+          [severity](MessageFixedText &x) { x.set_severity(severity); },
+          [severity](MessageFormattedText &x) { x.set_severity(severity); },
+      },
+      text_);
+  return *this;
+}
+
 std::string Message::ToString() const {
   return std::visit(
       common::visitors{

       location_);
 }
 
-void Message::Emit(llvm::raw_ostream &o, const AllCookedSources &allCooked,
-    bool echoSourceLine) const {
-  std::optional<ProvenanceRange> provenanceRange{GetProvenanceRange(allCooked)};
-  std::string text;
-  switch (severity()) {
+static std::string Prefix(Severity severity) {
+  switch (severity) {
   case Severity::Error:
-    text = "error: ";
-    break;
+    return "error: ";
   case Severity::Warning:
-    text = "warning: ";
-    break;
+    return "warning: ";
   case Severity::Portability:
-    text = "portability: ";
-    break;
+    return "portability: ";
+  case Severity::Because:
+    return "because: ";
+  case Severity::Context:
+    return "in the context: ";
   case Severity::None:
     break;
   }
-  text += ToString();
+  return "";
+}
+
+void Message::Emit(llvm::raw_ostream &o, const AllCookedSources &allCooked,
+    bool echoSourceLine) const {
+  std::optional<ProvenanceRange> provenanceRange{GetProvenanceRange(allCooked)};
   const AllSources &sources{allCooked.allSources()};
-  sources.EmitMessage(o, provenanceRange, text, echoSourceLine);
+  sources.EmitMessage(
+      o, provenanceRange, Prefix(severity()) + ToString(), echoSourceLine);
   bool isContext{attachmentIsContext_};
   for (const Message *attachment{attachment_.get()}; attachment;
        attachment = attachment->attachment_.get()) {
-    text.clear();
-    if (isContext) {
-      text = "in the context: ";
-    }
-    text += attachment->ToString();
-    sources.EmitMessage(
-        o, attachment->GetProvenanceRange(allCooked), text, echoSourceLine);
-    isContext = attachment->attachmentIsContext_;
+    sources.EmitMessage(o, attachment->GetProvenanceRange(allCooked),
+        Prefix(isContext ? Severity::Context : attachment->severity()) +
+            attachment->ToString(),
+        echoSourceLine);
   }
 }
 
 // Messages are equal if they're for the same location and text, and the user
 // visible aspects of their attachments are the same
 bool Message::operator==(const Message &that) const {
-  if (!AtSameLocation(that) || ToString() != that.ToString()) {
+  if (!AtSameLocation(that) || ToString() != that.ToString() ||
+      severity() != that.severity() ||
+      attachmentIsContext_ != that.attachmentIsContext_) {
     return false;
   }
   const Message *thatAttachment{that.attachment_.get()};
   for (const Message *attachment{attachment_.get()}; attachment;
        attachment = attachment->attachment_.get()) {
-    if (!thatAttachment ||
-        attachment->attachmentIsContext_ !=
-            thatAttachment->attachmentIsContext_ ||
-        *attachment != *thatAttachment) {
+    if (!thatAttachment || !attachment->AtSameLocation(*thatAttachment) ||
+        attachment->ToString() != thatAttachment->ToString() ||
+        attachment->severity() != thatAttachment->severity()) {
       return false;
     }
     thatAttachment = thatAttachment->attachment_.get();
   }
-  return true;
+  return !thatAttachment;
 }
 
 bool Message::Merge(const Message &that) {

   }
 }
 
-void Messages::AttachTo(Message &msg) {
+void Messages::AttachTo(Message &msg, std::optional<Severity> severity) {
   for (Message &m : messages_) {
-    msg.Attach(std::move(m));
+    Message m2{std::move(m)};
+    if (severity) {
+      m2.set_severity(*severity);
+    }
+    msg.Attach(std::move(m2));
   }
   messages_.clear();
 }

flang/lib/Semantics/check-call.cpp

     if (treatingExternalAsImplicit && !buffer.empty()) {
       if (auto *msg{messages.Say(
               "If the procedure's interface were explicit, this reference would be in error:"_warn_en_US)}) {
-        buffer.AttachTo(*msg);
+        buffer.AttachTo(*msg, parser::Severity::Because);
       }
     }
     if (auto *msgs{messages.messages()}) {

flang/lib/Semantics/check-select-rank.cpp

                     .Say(rankCaseStmt.source,
                         "Not more than one of the selectors of SELECT RANK "
                         "statement may be DEFAULT"_err_en_US)
-                    .Attach(prevLocDefault, "Previous use"_err_en_US);
+                    .Attach(prevLocDefault, "Previous use"_en_US);
               }
             },
             [&](const parser::Star &) { // C1153

                     .Say(rankCaseStmt.source,
                         "Not more than one of the selectors of SELECT RANK "
                         "statement may be '*'"_err_en_US)
-                    .Attach(prevLocStar, "Previous use"_err_en_US);
+                    .Attach(prevLocStar, "Previous use"_en_US);
               }
               if (saveSelSymbol &&
                   IsAllocatableOrPointer(*saveSelSymbol)) { // C1155

                         .Say(rankCaseStmt.source,
                             "Same rank value (%d) not allowed more than once"_err_en_US,
                             *val)
-                        .Attach(prevloc, "Previous use"_err_en_US);
+                        .Attach(prevloc, "Previous use"_en_US);
                   }
                 }
               }

flang/lib/Semantics/expression.cpp

           symbol->name())};
       if (subp->isFunction()) {
         const auto &result{subp->result().name()};
-        msg->Attach(result, "Function result is '%s'"_err_en_US, result);
+        msg->Attach(result, "Function result is '%s'"_en_US, result);
       }
     } else {
       context_.SayAt(x, "Assignment to constant '%s' is not allowed"_err_en_US,

flang/lib/Semantics/resolve-names.cpp

     if (const auto *details{prev.detailsIf<UseDetails>()}) {
       Say(name, "'%s' is already declared in this scoping unit"_err_en_US)
           .Attach(details->location(),
-              "It is use-associated with '%s' in module '%s'"_err_en_US,
+              "It is use-associated with '%s' in module '%s'"_en_US,
               details->symbol().name(), GetUsedModule(*details).name());
     } else {
       SayAlreadyDeclared(name, prev.name());

flang/test/Semantics/call25.f90

   call subr3(explicitLength)
   call subr3(assumedLength)
   !CHECK: warning: If the procedure's interface were explicit, this reference would be in error:
-  !CHECK: Actual argument function associated with procedure dummy argument 'f=' has incompatible result type
+  !CHECK: because: Actual argument function associated with procedure dummy argument 'f=' has incompatible result type
   call subr3(notChar)
 end program