[flang-commits] [flang] ef141ae - [flang] Improve appearance of message attachments
Peter Klausler via flang-commits
flang-commits at lists.llvm.org
Thu Apr 14 07:35:02 PDT 2022
Author: Peter Klausler
Date: 2022-04-14T07:34:50-07:00
New Revision: ef141aec3c81b33bd2022f258e6ca8d4b1611fd3
URL: https://github.com/llvm/llvm-project/commit/ef141aec3c81b33bd2022f258e6ca8d4b1611fd3
DIFF: https://github.com/llvm/llvm-project/commit/ef141aec3c81b33bd2022f258e6ca8d4b1611fd3.diff
LOG: [flang] Improve appearance of message attachments
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.
Differential Revision: https://reviews.llvm.org/D123710
Added:
Modified:
flang/include/flang/Parser/message.h
flang/lib/Parser/message.cpp
flang/lib/Semantics/check-call.cpp
flang/lib/Semantics/check-select-rank.cpp
flang/lib/Semantics/expression.cpp
flang/lib/Semantics/resolve-names.cpp
flang/test/Semantics/call25.f90
Removed:
################################################################################
diff --git a/flang/include/flang/Parser/message.h b/flang/include/flang/Parser/message.h
index 432ee36862d4c..80a03401feea5 100644
--- a/flang/include/flang/Parser/message.h
+++ b/flang/include/flang/Parser/message.h
@@ -29,9 +29,16 @@
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:
@@ -46,6 +53,10 @@ class MessageFixedText {
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:
@@ -90,6 +101,10 @@ class MessageFormattedText {
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:
@@ -183,7 +198,6 @@ class Message : public common::ReferenceCounted<Message> {
: 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) {
@@ -199,6 +213,7 @@ class Message : public common::ReferenceCounted<Message> {
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;
@@ -252,7 +267,7 @@ class Messages {
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:
diff --git a/flang/lib/Parser/message.cpp b/flang/lib/Parser/message.cpp
index 63515fd2e0fa5..522101f9c2300 100644
--- a/flang/lib/Parser/message.cpp
+++ b/flang/lib/Parser/message.cpp
@@ -167,6 +167,17 @@ Severity Message::severity() const {
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{
@@ -201,58 +212,59 @@ std::optional<ProvenanceRange> Message::GetProvenanceRange(
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) {
@@ -360,9 +372,13 @@ void Messages::Emit(llvm::raw_ostream &o, const AllCookedSources &allCooked,
}
}
-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();
}
diff --git a/flang/lib/Semantics/check-call.cpp b/flang/lib/Semantics/check-call.cpp
index ad3c5cb434d4f..9f4970c51521e 100644
--- a/flang/lib/Semantics/check-call.cpp
+++ b/flang/lib/Semantics/check-call.cpp
@@ -892,7 +892,7 @@ void CheckArguments(const characteristics::Procedure &proc,
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()}) {
diff --git a/flang/lib/Semantics/check-select-rank.cpp b/flang/lib/Semantics/check-select-rank.cpp
index 595c17fa7211a..d900584b414c7 100644
--- a/flang/lib/Semantics/check-select-rank.cpp
+++ b/flang/lib/Semantics/check-select-rank.cpp
@@ -71,7 +71,7 @@ void SelectRankConstructChecker::Leave(
.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
@@ -83,7 +83,7 @@ void SelectRankConstructChecker::Leave(
.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
@@ -111,7 +111,7 @@ void SelectRankConstructChecker::Leave(
.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);
}
}
}
diff --git a/flang/lib/Semantics/expression.cpp b/flang/lib/Semantics/expression.cpp
index 342f34164f176..f28cf9800a41c 100644
--- a/flang/lib/Semantics/expression.cpp
+++ b/flang/lib/Semantics/expression.cpp
@@ -3199,7 +3199,7 @@ void ArgumentAnalyzer::Analyze(const parser::Variable &x) {
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,
diff --git a/flang/lib/Semantics/resolve-names.cpp b/flang/lib/Semantics/resolve-names.cpp
index 5b63f842d63f3..8eff236a4a0d1 100644
--- a/flang/lib/Semantics/resolve-names.cpp
+++ b/flang/lib/Semantics/resolve-names.cpp
@@ -1979,7 +1979,7 @@ void ScopeHandler::SayAlreadyDeclared(const SourceName &name, Symbol &prev) {
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());
diff --git a/flang/test/Semantics/call25.f90 b/flang/test/Semantics/call25.f90
index 66bd7fcc5da4f..0f9219f30694f 100644
--- a/flang/test/Semantics/call25.f90
+++ b/flang/test/Semantics/call25.f90
@@ -44,6 +44,6 @@ program main
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
More information about the flang-commits
mailing list