From b9cdd2e72c9c9e6b10098c4144b3e840122b56d6 Mon Sep 17 00:00:00 2001 From: Andre Kuhlenschmidt Date: Mon, 14 Jul 2025 15:56:39 -0700 Subject: [PATCH 1/6] initial commit --- flang/include/flang/Parser/message.h | 37 ++++++++++++++------- flang/lib/Frontend/FrontendAction.cpp | 6 ++-- flang/lib/Parser/message.cpp | 37 +++++++++++++-------- flang/lib/Semantics/resolve-names.cpp | 3 +- flang/lib/Semantics/semantics.cpp | 5 ++- flang/test/Driver/color-diagnostics-scan.f | 16 ++++----- flang/test/Driver/fatal-errors-warnings.f90 | 32 ++++++++++++++++++ 7 files changed, 96 insertions(+), 40 deletions(-) create mode 100644 flang/test/Driver/fatal-errors-warnings.f90 diff --git a/flang/include/flang/Parser/message.h b/flang/include/flang/Parser/message.h index db1a0a65157e3..d1d313a70a016 100644 --- a/flang/include/flang/Parser/message.h +++ b/flang/include/flang/Parser/message.h @@ -56,13 +56,19 @@ class MessageFixedText { CharBlock text() const { return text_; } bool empty() const { return text_.empty(); } - Severity severity() const { return severity_; } + Severity severity(bool warningsAreErrors = false) const { + if (warningsAreErrors) { + return Severity::Error; + } + return severity_; + } MessageFixedText &set_severity(Severity severity) { severity_ = severity; return *this; } - bool IsFatal() const { - return severity_ == Severity::Error || severity_ == Severity::Todo; + bool IsFatal(bool warningsAreErrors = false) const { + Severity sev{severity(warningsAreErrors)}; + return sev == Severity::Error || sev == Severity::Todo; } private: @@ -105,7 +111,7 @@ class MessageFormattedText { public: template MessageFormattedText(const MessageFixedText &text, A &&...x) - : severity_{text.severity()} { + : severity_{text.severity(false)} { Format(&text, Convert(std::forward(x))...); } MessageFormattedText(const MessageFormattedText &) = default; @@ -113,14 +119,20 @@ class MessageFormattedText { MessageFormattedText &operator=(const MessageFormattedText &) = default; MessageFormattedText &operator=(MessageFormattedText &&) = default; const std::string &string() const { return string_; } - bool IsFatal() const { - return severity_ == Severity::Error || severity_ == Severity::Todo; + Severity severity(bool warningsAreErrors = false) const { + if (warningsAreErrors) { + return Severity::Error; + } + return severity_; } - Severity severity() const { return severity_; } MessageFormattedText &set_severity(Severity severity) { severity_ = severity; return *this; } + bool IsFatal(bool warningsAreErrors = false) const { + Severity sev{severity(warningsAreErrors)}; + return sev == Severity::Error || sev == Severity::Todo; + } std::string MoveString() { return std::move(string_); } bool operator==(const MessageFormattedText &that) const { return severity_ == that.severity_ && string_ == that.string_; @@ -281,8 +293,8 @@ class Message : public common::ReferenceCounted { } bool SortBefore(const Message &that) const; - bool IsFatal() const; - Severity severity() const; + bool IsFatal(bool warningsAreErrors = false) const; + Severity severity(bool warningsAreErrors = false) const; Message &set_severity(Severity); std::optional languageFeature() const; Message &set_languageFeature(common::LanguageFeature); @@ -293,7 +305,8 @@ class Message : public common::ReferenceCounted { const AllCookedSources &) const; void Emit(llvm::raw_ostream &, const AllCookedSources &, bool echoSourceLine = true, - const common::LanguageFeatureControl *hintFlags = nullptr) const; + const common::LanguageFeatureControl *hintFlags = nullptr, + bool warningsAreErrors = false) const; // If this Message or any of its attachments locates itself via a CharBlock, // replace its location with the corresponding ProvenanceRange. @@ -355,9 +368,9 @@ class Messages { void Emit(llvm::raw_ostream &, const AllCookedSources &, bool echoSourceLines = true, const common::LanguageFeatureControl *hintFlags = nullptr, - std::size_t maxErrorsToEmit = 0) const; + std::size_t maxErrorsToEmit = 0, bool warningsAreErrors = false) const; void AttachTo(Message &, std::optional = std::nullopt); - bool AnyFatalError() const; + bool AnyFatalError(bool warningsAreErrors = false) const; private: std::list messages_; diff --git a/flang/lib/Frontend/FrontendAction.cpp b/flang/lib/Frontend/FrontendAction.cpp index 2429e07e5b8c4..d32f477445f93 100644 --- a/flang/lib/Frontend/FrontendAction.cpp +++ b/flang/lib/Frontend/FrontendAction.cpp @@ -238,7 +238,8 @@ bool FrontendAction::reportFatalErrors(const char (&message)[N]) { instance->getDiagnostics().Report(diagID) << getCurrentFileOrBufferName(); instance->getParsing().messages().Emit( llvm::errs(), instance->getAllCookedSources(), - /*echoSourceLines=*/true, &features, maxErrors); + /*echoSourceLines=*/true, &features, maxErrors, + instance->getInvocation().getWarnAsErr()); return true; } if (instance->getParsing().parseTree().has_value() && @@ -249,7 +250,8 @@ bool FrontendAction::reportFatalErrors(const char (&message)[N]) { instance->getDiagnostics().Report(diagID) << getCurrentFileOrBufferName(); instance->getParsing().messages().Emit( llvm::errs(), instance->getAllCookedSources(), - /*echoSourceLine=*/true, &features, maxErrors); + /*echoSourceLine=*/true, &features, maxErrors, + instance->getInvocation().getWarnAsErr()); instance->getParsing().EmitMessage( llvm::errs(), instance->getParsing().finalRestingPlace(), "parser FAIL (final position)", "error: ", llvm::raw_ostream::RED); diff --git a/flang/lib/Parser/message.cpp b/flang/lib/Parser/message.cpp index 909fba948a45a..574e9f5d424f8 100644 --- a/flang/lib/Parser/message.cpp +++ b/flang/lib/Parser/message.cpp @@ -161,16 +161,21 @@ bool Message::SortBefore(const Message &that) const { location_, that.location_); } -bool Message::IsFatal() const { - return severity() == Severity::Error || severity() == Severity::Todo; +bool Message::IsFatal(bool warningsAreErrors) const { + Severity sev{severity(warningsAreErrors)}; + return sev == Severity::Error || sev == Severity::Todo; } -Severity Message::severity() const { +Severity Message::severity(bool warningsAreErrors) const { return common::visit( common::visitors{ [](const MessageExpectedText &) { return Severity::Error; }, - [](const MessageFixedText &x) { return x.severity(); }, - [](const MessageFormattedText &x) { return x.severity(); }, + [=](const MessageFixedText &x) { + return x.severity(warningsAreErrors); + }, + [=](const MessageFormattedText &x) { + return x.severity(warningsAreErrors); + }, }, text_); } @@ -295,15 +300,16 @@ static constexpr int MAX_CONTEXTS_EMITTED{2}; static constexpr bool OMIT_SHARED_CONTEXTS{true}; void Message::Emit(llvm::raw_ostream &o, const AllCookedSources &allCooked, - bool echoSourceLine, - const common::LanguageFeatureControl *hintFlagPtr) const { + bool echoSourceLine, const common::LanguageFeatureControl *hintFlagPtr, + bool warningsAreErrors) const { std::optional provenanceRange{GetProvenanceRange(allCooked)}; const AllSources &sources{allCooked.allSources()}; const std::string text{ToString()}; + Severity sev{severity(warningsAreErrors)}; const std::string hint{ HintLanguageControlFlag(hintFlagPtr, languageFeature_, usageWarning_)}; - sources.EmitMessage(o, provenanceRange, text + hint, Prefix(severity()), - PrefixColor(severity()), echoSourceLine); + sources.EmitMessage(o, provenanceRange, text + hint, Prefix(sev), + PrefixColor(sev), echoSourceLine); // Refers to whether the attachment in the loop below is a context, but can't // be declared inside the loop because the previous iteration's // attachment->attachmentIsContext_ indicates this. @@ -453,7 +459,7 @@ void Messages::ResolveProvenances(const AllCookedSources &allCooked) { void Messages::Emit(llvm::raw_ostream &o, const AllCookedSources &allCooked, bool echoSourceLines, const common::LanguageFeatureControl *hintFlagPtr, - std::size_t maxErrorsToEmit) const { + std::size_t maxErrorsToEmit, bool warningsAreErrors) const { std::vector sorted; for (const auto &msg : messages_) { sorted.push_back(&msg); @@ -467,9 +473,9 @@ void Messages::Emit(llvm::raw_ostream &o, const AllCookedSources &allCooked, // Don't emit two identical messages for the same location continue; } - msg->Emit(o, allCooked, echoSourceLines, hintFlagPtr); + msg->Emit(o, allCooked, echoSourceLines, hintFlagPtr, warningsAreErrors); lastMsg = msg; - if (msg->IsFatal()) { + if (msg->IsFatal(warningsAreErrors)) { ++errorsEmitted; } // If maxErrorsToEmit is 0, emit all errors, otherwise break after @@ -491,9 +497,12 @@ void Messages::AttachTo(Message &msg, std::optional severity) { messages_.clear(); } -bool Messages::AnyFatalError() const { +bool Messages::AnyFatalError(bool warningsAreErrors) const { + if (messages_.empty()) { + return false; + } for (const auto &msg : messages_) { - if (msg.IsFatal()) { + if (msg.IsFatal(warningsAreErrors)) { return true; } } diff --git a/flang/lib/Semantics/resolve-names.cpp b/flang/lib/Semantics/resolve-names.cpp index 96faa5fd954cd..df898f6e5651e 100644 --- a/flang/lib/Semantics/resolve-names.cpp +++ b/flang/lib/Semantics/resolve-names.cpp @@ -7575,7 +7575,8 @@ bool DeclarationVisitor::OkToAddComponent( if (msg) { auto &said{Say2(name, std::move(*msg), *prev, "Previous declaration of '%s'"_en_US)}; - if (msg->severity() == parser::Severity::Error) { + if (msg->severity(/*warningsAreErrors=*/false) == + parser::Severity::Error) { Resolve(name, *prev); return false; } diff --git a/flang/lib/Semantics/semantics.cpp b/flang/lib/Semantics/semantics.cpp index ab78605d01f4c..b15ed057b52f2 100644 --- a/flang/lib/Semantics/semantics.cpp +++ b/flang/lib/Semantics/semantics.cpp @@ -376,8 +376,7 @@ const DeclTypeSpec &SemanticsContext::MakeLogicalType(int kind) { } bool SemanticsContext::AnyFatalError() const { - return !messages_.empty() && - (warningsAreErrors_ || messages_.AnyFatalError()); + return messages_.AnyFatalError(warningsAreErrors_); } bool SemanticsContext::HasError(const Symbol &symbol) { return errorSymbols_.count(symbol) > 0; @@ -658,7 +657,7 @@ void Semantics::EmitMessages(llvm::raw_ostream &os) { context_.messages().ResolveProvenances(context_.allCookedSources()); context_.messages().Emit(os, context_.allCookedSources(), /*echoSourceLine=*/true, &context_.languageFeatures(), - /*maxErrorsToEmit=*/context_.maxErrors()); + context_.maxErrors(), context_.warningsAreErrors()); } void SemanticsContext::DumpSymbols(llvm::raw_ostream &os) { diff --git a/flang/test/Driver/color-diagnostics-scan.f b/flang/test/Driver/color-diagnostics-scan.f index 29d4635b4fb03..2f647c923c2e6 100644 --- a/flang/test/Driver/color-diagnostics-scan.f +++ b/flang/test/Driver/color-diagnostics-scan.f @@ -3,24 +3,24 @@ ! Windows command prompt doesn't support ANSI escape sequences. ! REQUIRES: shell -! RUN: not %flang %s -E -Werror -fcolor-diagnostics 2>&1 \ +! RUN: %flang %s -E -fcolor-diagnostics 2>&1 \ ! RUN: | FileCheck %s --check-prefix=CHECK_CD -! RUN: not %flang %s -E -Werror -fno-color-diagnostics 2>&1 \ +! RUN: %flang %s -E -fno-color-diagnostics 2>&1 \ ! RUN: | FileCheck %s --check-prefix=CHECK_NCD -! RUN: not %flang_fc1 -E -Werror %s -fcolor-diagnostics 2>&1 \ +! RUN: %flang_fc1 -E %s -fcolor-diagnostics 2>&1 \ ! RUN: | FileCheck %s --check-prefix=CHECK_CD -! RUN: not %flang %s -E -Werror -fdiagnostics-color 2>&1 \ +! RUN: %flang %s -E -fdiagnostics-color 2>&1 \ ! RUN: | FileCheck %s --check-prefix=CHECK_CD -! RUN: not %flang %s -E -Werror -fno-diagnostics-color 2>&1 \ +! RUN: %flang %s -E -fno-diagnostics-color 2>&1 \ ! RUN: | FileCheck %s --check-prefix=CHECK_NCD -! RUN: not %flang %s -E -Werror -fdiagnostics-color=always 2>&1 \ +! RUN: %flang %s -E -fdiagnostics-color=always 2>&1 \ ! RUN: | FileCheck %s --check-prefix=CHECK_CD -! RUN: not %flang %s -E -Werror -fdiagnostics-color=never 2>&1 \ +! RUN: %flang %s -E -fdiagnostics-color=never 2>&1 \ ! RUN: | FileCheck %s --check-prefix=CHECK_NCD -! RUN: not %flang_fc1 -E -Werror %s 2>&1 | FileCheck %s --check-prefix=CHECK_NCD +! RUN: %flang_fc1 -E %s 2>&1 | FileCheck %s --check-prefix=CHECK_NCD ! CHECK_CD: {{.*}}[0;1;35mwarning: {{.*}}[0mCharacter in fixed-form label field must be a digit diff --git a/flang/test/Driver/fatal-errors-warnings.f90 b/flang/test/Driver/fatal-errors-warnings.f90 new file mode 100644 index 0000000000000..b4b23230587a4 --- /dev/null +++ b/flang/test/Driver/fatal-errors-warnings.f90 @@ -0,0 +1,32 @@ +! RUN: not %flang_fc1 -Wfatal-errors -pedantic %s 2>&1 | FileCheck %s --check-prefix=CHECK1 +! RUN: not %flang_fc1 -pedantic -Werror %s 2>&1 | FileCheck %s --check-prefix=CHECK2 +! RUN: not %flang_fc1 -Wfatal-errors -pedantic -Werror %s 2>&1 | FileCheck %s --check-prefix=CHECK3 + +module m + contains + subroutine foo(a) + real, intent(in), target :: a(:) + end subroutine +end module + +program test + use m + real, target :: a(1) + real :: b(1) + call foo(a) ! ok + !CHECK1: fatal-errors-warnings.f90:{{.*}} warning: + !CHECK2: fatal-errors-warnings.f90:{{.*}} error: + !CHECK3: fatal-errors-warnings.f90:{{.*}} error: + call foo(b) + !CHECK1: fatal-errors-warnings.f90:{{.*}} warning: + !CHECK2: fatal-errors-warnings.f90:{{.*}} error: + !CHECK3-NOT: error: + !CHECK3-NOT: warning: + call foo((a)) + !CHECK1: fatal-errors-warnings.f90:{{.*}} warning: + !CHECK2: fatal-errors-warnings.f90:{{.*}} error: + call foo(a([1])) + !CHECK1: fatal-errors-warnings.f90:{{.*}} error: + !CHECK2: fatal-errors-warnings.f90:{{.*}} error: + call foo(a(1)) +end \ No newline at end of file From 31d79c710a3a0a727bcf2d94e73280a9f62bb0ca Mon Sep 17 00:00:00 2001 From: Andre Kuhlenschmidt Date: Tue, 15 Jul 2025 09:21:19 -0700 Subject: [PATCH 2/6] don't display warnings as errors --- flang/lib/Parser/message.cpp | 6 +++++- flang/test/Driver/color-diagnostics-scan.f | 16 ++++++++-------- flang/test/Driver/fatal-errors-warnings.f90 | 15 +++++++-------- 3 files changed, 20 insertions(+), 17 deletions(-) diff --git a/flang/lib/Parser/message.cpp b/flang/lib/Parser/message.cpp index 574e9f5d424f8..6644fafffc1bc 100644 --- a/flang/lib/Parser/message.cpp +++ b/flang/lib/Parser/message.cpp @@ -473,7 +473,11 @@ void Messages::Emit(llvm::raw_ostream &o, const AllCookedSources &allCooked, // Don't emit two identical messages for the same location continue; } - msg->Emit(o, allCooked, echoSourceLines, hintFlagPtr, warningsAreErrors); + // We think it is confusing to users to display warnings and other + // diagnostics as errors, instead we just treat them as errors for the + // purpose of failing. + msg->Emit(o, allCooked, echoSourceLines, hintFlagPtr, + /*warningsAreErrors=*/false); lastMsg = msg; if (msg->IsFatal(warningsAreErrors)) { ++errorsEmitted; diff --git a/flang/test/Driver/color-diagnostics-scan.f b/flang/test/Driver/color-diagnostics-scan.f index 2f647c923c2e6..29d4635b4fb03 100644 --- a/flang/test/Driver/color-diagnostics-scan.f +++ b/flang/test/Driver/color-diagnostics-scan.f @@ -3,24 +3,24 @@ ! Windows command prompt doesn't support ANSI escape sequences. ! REQUIRES: shell -! RUN: %flang %s -E -fcolor-diagnostics 2>&1 \ +! RUN: not %flang %s -E -Werror -fcolor-diagnostics 2>&1 \ ! RUN: | FileCheck %s --check-prefix=CHECK_CD -! RUN: %flang %s -E -fno-color-diagnostics 2>&1 \ +! RUN: not %flang %s -E -Werror -fno-color-diagnostics 2>&1 \ ! RUN: | FileCheck %s --check-prefix=CHECK_NCD -! RUN: %flang_fc1 -E %s -fcolor-diagnostics 2>&1 \ +! RUN: not %flang_fc1 -E -Werror %s -fcolor-diagnostics 2>&1 \ ! RUN: | FileCheck %s --check-prefix=CHECK_CD -! RUN: %flang %s -E -fdiagnostics-color 2>&1 \ +! RUN: not %flang %s -E -Werror -fdiagnostics-color 2>&1 \ ! RUN: | FileCheck %s --check-prefix=CHECK_CD -! RUN: %flang %s -E -fno-diagnostics-color 2>&1 \ +! RUN: not %flang %s -E -Werror -fno-diagnostics-color 2>&1 \ ! RUN: | FileCheck %s --check-prefix=CHECK_NCD -! RUN: %flang %s -E -fdiagnostics-color=always 2>&1 \ +! RUN: not %flang %s -E -Werror -fdiagnostics-color=always 2>&1 \ ! RUN: | FileCheck %s --check-prefix=CHECK_CD -! RUN: %flang %s -E -fdiagnostics-color=never 2>&1 \ +! RUN: not %flang %s -E -Werror -fdiagnostics-color=never 2>&1 \ ! RUN: | FileCheck %s --check-prefix=CHECK_NCD -! RUN: %flang_fc1 -E %s 2>&1 | FileCheck %s --check-prefix=CHECK_NCD +! RUN: not %flang_fc1 -E -Werror %s 2>&1 | FileCheck %s --check-prefix=CHECK_NCD ! CHECK_CD: {{.*}}[0;1;35mwarning: {{.*}}[0mCharacter in fixed-form label field must be a digit diff --git a/flang/test/Driver/fatal-errors-warnings.f90 b/flang/test/Driver/fatal-errors-warnings.f90 index b4b23230587a4..2de09c3ed0778 100644 --- a/flang/test/Driver/fatal-errors-warnings.f90 +++ b/flang/test/Driver/fatal-errors-warnings.f90 @@ -1,4 +1,4 @@ -! RUN: not %flang_fc1 -Wfatal-errors -pedantic %s 2>&1 | FileCheck %s --check-prefix=CHECK1 +! RUN: %flang_fc1 -Wfatal-errors -pedantic %s 2>&1 | FileCheck %s --check-prefix=CHECK1 ! RUN: not %flang_fc1 -pedantic -Werror %s 2>&1 | FileCheck %s --check-prefix=CHECK2 ! RUN: not %flang_fc1 -Wfatal-errors -pedantic -Werror %s 2>&1 | FileCheck %s --check-prefix=CHECK3 @@ -15,18 +15,17 @@ program test real :: b(1) call foo(a) ! ok !CHECK1: fatal-errors-warnings.f90:{{.*}} warning: - !CHECK2: fatal-errors-warnings.f90:{{.*}} error: - !CHECK3: fatal-errors-warnings.f90:{{.*}} error: + !CHECK2: fatal-errors-warnings.f90:{{.*}} warning: + !CHECK3: fatal-errors-warnings.f90:{{.*}} warning: call foo(b) !CHECK1: fatal-errors-warnings.f90:{{.*}} warning: - !CHECK2: fatal-errors-warnings.f90:{{.*}} error: + !CHECK2: fatal-errors-warnings.f90:{{.*}} warning: !CHECK3-NOT: error: !CHECK3-NOT: warning: call foo((a)) !CHECK1: fatal-errors-warnings.f90:{{.*}} warning: - !CHECK2: fatal-errors-warnings.f90:{{.*}} error: + !CHECK2: fatal-errors-warnings.f90:{{.*}} warning: call foo(a([1])) - !CHECK1: fatal-errors-warnings.f90:{{.*}} error: - !CHECK2: fatal-errors-warnings.f90:{{.*}} error: - call foo(a(1)) + !! Hard error instead of warning if uncommented. + !call foo(a(1)) end \ No newline at end of file From 892e255be088b117b94203b66d75688fcd3ebc28 Mon Sep 17 00:00:00 2001 From: Andre Kuhlenschmidt Date: Tue, 15 Jul 2025 11:07:22 -0700 Subject: [PATCH 3/6] add EffectiveSeverity(...) --- flang/include/flang/Parser/message.h | 42 +++++++++++++++----------- flang/lib/Frontend/FrontendAction.cpp | 11 +++---- flang/lib/Parser/message.cpp | 43 ++++++++++++++++----------- flang/lib/Semantics/resolve-names.cpp | 3 +- 4 files changed, 54 insertions(+), 45 deletions(-) diff --git a/flang/include/flang/Parser/message.h b/flang/include/flang/Parser/message.h index d1d313a70a016..423856b084b92 100644 --- a/flang/include/flang/Parser/message.h +++ b/flang/include/flang/Parser/message.h @@ -56,18 +56,22 @@ class MessageFixedText { CharBlock text() const { return text_; } bool empty() const { return text_.empty(); } - Severity severity(bool warningsAreErrors = false) const { - if (warningsAreErrors) { - return Severity::Error; - } - return severity_; - } + Severity severity() const { return severity_; } MessageFixedText &set_severity(Severity severity) { severity_ = severity; return *this; } + // This is currently emulating how the frontend is currently handling other + // diagnostic messages. We may want to be more nuanced in the future. + Severity EffectiveSeverity(bool warningsAreErrors) const { + if (warningsAreErrors) { + return Severity::Error; + } else { + return severity_; + } + } bool IsFatal(bool warningsAreErrors = false) const { - Severity sev{severity(warningsAreErrors)}; + Severity sev{EffectiveSeverity(warningsAreErrors)}; return sev == Severity::Error || sev == Severity::Todo; } @@ -111,7 +115,7 @@ class MessageFormattedText { public: template MessageFormattedText(const MessageFixedText &text, A &&...x) - : severity_{text.severity(false)} { + : severity_{text.severity()} { Format(&text, Convert(std::forward(x))...); } MessageFormattedText(const MessageFormattedText &) = default; @@ -119,18 +123,20 @@ class MessageFormattedText { MessageFormattedText &operator=(const MessageFormattedText &) = default; MessageFormattedText &operator=(MessageFormattedText &&) = default; const std::string &string() const { return string_; } - Severity severity(bool warningsAreErrors = false) const { - if (warningsAreErrors) { - return Severity::Error; - } - return severity_; - } + Severity severity() const { return severity_; } MessageFormattedText &set_severity(Severity severity) { severity_ = severity; return *this; } + Severity EffectiveSeverity(bool warningsAreErrors) const { + if (warningsAreErrors) { + return Severity::Error; + } else { + return severity_; + } + } bool IsFatal(bool warningsAreErrors = false) const { - Severity sev{severity(warningsAreErrors)}; + Severity sev{EffectiveSeverity(warningsAreErrors)}; return sev == Severity::Error || sev == Severity::Todo; } std::string MoveString() { return std::move(string_); } @@ -294,7 +300,8 @@ class Message : public common::ReferenceCounted { bool SortBefore(const Message &that) const; bool IsFatal(bool warningsAreErrors = false) const; - Severity severity(bool warningsAreErrors = false) const; + Severity EffectiveSeverity(bool warningsAreErrors) const; + Severity severity() const; Message &set_severity(Severity); std::optional languageFeature() const; Message &set_languageFeature(common::LanguageFeature); @@ -305,8 +312,7 @@ class Message : public common::ReferenceCounted { const AllCookedSources &) const; void Emit(llvm::raw_ostream &, const AllCookedSources &, bool echoSourceLine = true, - const common::LanguageFeatureControl *hintFlags = nullptr, - bool warningsAreErrors = false) const; + const common::LanguageFeatureControl *hintFlags = nullptr) const; // If this Message or any of its attachments locates itself via a CharBlock, // replace its location with the corresponding ProvenanceRange. diff --git a/flang/lib/Frontend/FrontendAction.cpp b/flang/lib/Frontend/FrontendAction.cpp index d32f477445f93..58901c6000380 100644 --- a/flang/lib/Frontend/FrontendAction.cpp +++ b/flang/lib/Frontend/FrontendAction.cpp @@ -230,16 +230,14 @@ bool FrontendAction::reportFatalErrors(const char (&message)[N]) { const common::LanguageFeatureControl &features{ instance->getInvocation().getFortranOpts().features}; const size_t maxErrors{instance->getInvocation().getMaxErrors()}; - if (!instance->getParsing().messages().empty() && - (instance->getInvocation().getWarnAsErr() || - instance->getParsing().messages().AnyFatalError())) { + const bool warningsAreErrors{instance->getInvocation().getWarnAsErr()}; + if (instance->getParsing().messages().AnyFatalError(warningsAreErrors)) { const unsigned diagID = instance->getDiagnostics().getCustomDiagID( clang::DiagnosticsEngine::Error, message); instance->getDiagnostics().Report(diagID) << getCurrentFileOrBufferName(); instance->getParsing().messages().Emit( llvm::errs(), instance->getAllCookedSources(), - /*echoSourceLines=*/true, &features, maxErrors, - instance->getInvocation().getWarnAsErr()); + /*echoSourceLines=*/true, &features, maxErrors, warningsAreErrors); return true; } if (instance->getParsing().parseTree().has_value() && @@ -250,8 +248,7 @@ bool FrontendAction::reportFatalErrors(const char (&message)[N]) { instance->getDiagnostics().Report(diagID) << getCurrentFileOrBufferName(); instance->getParsing().messages().Emit( llvm::errs(), instance->getAllCookedSources(), - /*echoSourceLine=*/true, &features, maxErrors, - instance->getInvocation().getWarnAsErr()); + /*echoSourceLine=*/true, &features, maxErrors, warningsAreErrors); instance->getParsing().EmitMessage( llvm::errs(), instance->getParsing().finalRestingPlace(), "parser FAIL (final position)", "error: ", llvm::raw_ostream::RED); diff --git a/flang/lib/Parser/message.cpp b/flang/lib/Parser/message.cpp index 6644fafffc1bc..80fe5b2a14ad2 100644 --- a/flang/lib/Parser/message.cpp +++ b/flang/lib/Parser/message.cpp @@ -161,25 +161,35 @@ bool Message::SortBefore(const Message &that) const { location_, that.location_); } -bool Message::IsFatal(bool warningsAreErrors) const { - Severity sev{severity(warningsAreErrors)}; - return sev == Severity::Error || sev == Severity::Todo; -} - -Severity Message::severity(bool warningsAreErrors) const { +Severity Message::EffectiveSeverity(bool warningsAreErrors) const { return common::visit( common::visitors{ [](const MessageExpectedText &) { return Severity::Error; }, [=](const MessageFixedText &x) { - return x.severity(warningsAreErrors); + return x.EffectiveSeverity(warningsAreErrors); }, [=](const MessageFormattedText &x) { - return x.severity(warningsAreErrors); + return x.EffectiveSeverity(warningsAreErrors); }, }, text_); } +bool Message::IsFatal(bool warningsAreErrors) const { + Severity sev{EffectiveSeverity(warningsAreErrors)}; + return sev == Severity::Error || sev == Severity::Todo; +} + +Severity Message::severity() const { + return common::visit( + common::visitors{ + [](const MessageExpectedText &) { return Severity::Error; }, + [](const MessageFixedText &x) { return x.severity(); }, + [](const MessageFormattedText &x) { return x.severity(); }, + }, + text_); +} + Message &Message::set_severity(Severity severity) { common::visit( common::visitors{ @@ -299,17 +309,18 @@ static std::string HintLanguageControlFlag( static constexpr int MAX_CONTEXTS_EMITTED{2}; static constexpr bool OMIT_SHARED_CONTEXTS{true}; +// We think it is confusing to users to display warnings and other +// diagnostics as errors. Don't use EffectiveSeverity() here. void Message::Emit(llvm::raw_ostream &o, const AllCookedSources &allCooked, - bool echoSourceLine, const common::LanguageFeatureControl *hintFlagPtr, - bool warningsAreErrors) const { + bool echoSourceLine, + const common::LanguageFeatureControl *hintFlagPtr) const { std::optional provenanceRange{GetProvenanceRange(allCooked)}; const AllSources &sources{allCooked.allSources()}; const std::string text{ToString()}; - Severity sev{severity(warningsAreErrors)}; const std::string hint{ HintLanguageControlFlag(hintFlagPtr, languageFeature_, usageWarning_)}; - sources.EmitMessage(o, provenanceRange, text + hint, Prefix(sev), - PrefixColor(sev), echoSourceLine); + sources.EmitMessage(o, provenanceRange, text + hint, Prefix(severity()), + PrefixColor(severity()), echoSourceLine); // Refers to whether the attachment in the loop below is a context, but can't // be declared inside the loop because the previous iteration's // attachment->attachmentIsContext_ indicates this. @@ -473,11 +484,7 @@ void Messages::Emit(llvm::raw_ostream &o, const AllCookedSources &allCooked, // Don't emit two identical messages for the same location continue; } - // We think it is confusing to users to display warnings and other - // diagnostics as errors, instead we just treat them as errors for the - // purpose of failing. - msg->Emit(o, allCooked, echoSourceLines, hintFlagPtr, - /*warningsAreErrors=*/false); + msg->Emit(o, allCooked, echoSourceLines, hintFlagPtr); lastMsg = msg; if (msg->IsFatal(warningsAreErrors)) { ++errorsEmitted; diff --git a/flang/lib/Semantics/resolve-names.cpp b/flang/lib/Semantics/resolve-names.cpp index df898f6e5651e..96faa5fd954cd 100644 --- a/flang/lib/Semantics/resolve-names.cpp +++ b/flang/lib/Semantics/resolve-names.cpp @@ -7575,8 +7575,7 @@ bool DeclarationVisitor::OkToAddComponent( if (msg) { auto &said{Say2(name, std::move(*msg), *prev, "Previous declaration of '%s'"_en_US)}; - if (msg->severity(/*warningsAreErrors=*/false) == - parser::Severity::Error) { + if (msg->severity() == parser::Severity::Error) { Resolve(name, *prev); return false; } From 6bfd9255636df484d88fd46fbc84c368085501b6 Mon Sep 17 00:00:00 2001 From: Andre Kuhlenschmidt Date: Tue, 15 Jul 2025 15:07:25 -0700 Subject: [PATCH 4/6] remove actual promotion --- flang/include/flang/Parser/message.h | 28 +++++----------------------- flang/lib/Parser/message.cpp | 26 +++++++------------------- 2 files changed, 12 insertions(+), 42 deletions(-) diff --git a/flang/include/flang/Parser/message.h b/flang/include/flang/Parser/message.h index 423856b084b92..839bca158a78a 100644 --- a/flang/include/flang/Parser/message.h +++ b/flang/include/flang/Parser/message.h @@ -61,18 +61,9 @@ class MessageFixedText { severity_ = severity; return *this; } - // This is currently emulating how the frontend is currently handling other - // diagnostic messages. We may want to be more nuanced in the future. - Severity EffectiveSeverity(bool warningsAreErrors) const { - if (warningsAreErrors) { - return Severity::Error; - } else { - return severity_; - } - } - bool IsFatal(bool warningsAreErrors = false) const { - Severity sev{EffectiveSeverity(warningsAreErrors)}; - return sev == Severity::Error || sev == Severity::Todo; + + bool IsFatal() const { + return severity() == Severity::Error || severity() == Severity::Todo; } private: @@ -128,16 +119,8 @@ class MessageFormattedText { severity_ = severity; return *this; } - Severity EffectiveSeverity(bool warningsAreErrors) const { - if (warningsAreErrors) { - return Severity::Error; - } else { - return severity_; - } - } bool IsFatal(bool warningsAreErrors = false) const { - Severity sev{EffectiveSeverity(warningsAreErrors)}; - return sev == Severity::Error || sev == Severity::Todo; + return severity() == Severity::Error || severity() == Severity::Todo; } std::string MoveString() { return std::move(string_); } bool operator==(const MessageFormattedText &that) const { @@ -299,8 +282,7 @@ class Message : public common::ReferenceCounted { } bool SortBefore(const Message &that) const; - bool IsFatal(bool warningsAreErrors = false) const; - Severity EffectiveSeverity(bool warningsAreErrors) const; + bool IsFatal() const; Severity severity() const; Message &set_severity(Severity); std::optional languageFeature() const; diff --git a/flang/lib/Parser/message.cpp b/flang/lib/Parser/message.cpp index 80fe5b2a14ad2..36d570408cdb9 100644 --- a/flang/lib/Parser/message.cpp +++ b/flang/lib/Parser/message.cpp @@ -161,23 +161,8 @@ bool Message::SortBefore(const Message &that) const { location_, that.location_); } -Severity Message::EffectiveSeverity(bool warningsAreErrors) const { - return common::visit( - common::visitors{ - [](const MessageExpectedText &) { return Severity::Error; }, - [=](const MessageFixedText &x) { - return x.EffectiveSeverity(warningsAreErrors); - }, - [=](const MessageFormattedText &x) { - return x.EffectiveSeverity(warningsAreErrors); - }, - }, - text_); -} - -bool Message::IsFatal(bool warningsAreErrors) const { - Severity sev{EffectiveSeverity(warningsAreErrors)}; - return sev == Severity::Error || sev == Severity::Todo; +bool Message::IsFatal() const { + return severity() == Severity::Error || severity() == Severity::Todo; } Severity Message::severity() const { @@ -486,7 +471,7 @@ void Messages::Emit(llvm::raw_ostream &o, const AllCookedSources &allCooked, } msg->Emit(o, allCooked, echoSourceLines, hintFlagPtr); lastMsg = msg; - if (msg->IsFatal(warningsAreErrors)) { + if (warningsAreErrors || msg->IsFatal()) { ++errorsEmitted; } // If maxErrorsToEmit is 0, emit all errors, otherwise break after @@ -512,8 +497,11 @@ bool Messages::AnyFatalError(bool warningsAreErrors) const { if (messages_.empty()) { return false; } + if (warningsAreErrors) { + return true; + } for (const auto &msg : messages_) { - if (msg.IsFatal(warningsAreErrors)) { + if (msg.IsFatal()) { return true; } } From 2693db8d3188c956204db8e70c8dc4ba97801875 Mon Sep 17 00:00:00 2001 From: Andre Kuhlenschmidt Date: Tue, 15 Jul 2025 15:15:55 -0700 Subject: [PATCH 5/6] clean up removal --- flang/include/flang/Parser/message.h | 9 ++++----- flang/lib/Parser/message.cpp | 2 -- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/flang/include/flang/Parser/message.h b/flang/include/flang/Parser/message.h index 839bca158a78a..9192d23529913 100644 --- a/flang/include/flang/Parser/message.h +++ b/flang/include/flang/Parser/message.h @@ -61,9 +61,8 @@ class MessageFixedText { severity_ = severity; return *this; } - bool IsFatal() const { - return severity() == Severity::Error || severity() == Severity::Todo; + return severity_ == Severity::Error || severity_ == Severity::Todo; } private: @@ -114,14 +113,14 @@ class MessageFormattedText { MessageFormattedText &operator=(const MessageFormattedText &) = default; MessageFormattedText &operator=(MessageFormattedText &&) = default; const std::string &string() const { return string_; } + bool IsFatal() const { + return severity_ == Severity::Error || severity_ == Severity::Todo; + } Severity severity() const { return severity_; } MessageFormattedText &set_severity(Severity severity) { severity_ = severity; return *this; } - bool IsFatal(bool warningsAreErrors = false) const { - return severity() == Severity::Error || severity() == Severity::Todo; - } std::string MoveString() { return std::move(string_); } bool operator==(const MessageFormattedText &that) const { return severity_ == that.severity_ && string_ == that.string_; diff --git a/flang/lib/Parser/message.cpp b/flang/lib/Parser/message.cpp index 36d570408cdb9..064494de9b478 100644 --- a/flang/lib/Parser/message.cpp +++ b/flang/lib/Parser/message.cpp @@ -294,8 +294,6 @@ static std::string HintLanguageControlFlag( static constexpr int MAX_CONTEXTS_EMITTED{2}; static constexpr bool OMIT_SHARED_CONTEXTS{true}; -// We think it is confusing to users to display warnings and other -// diagnostics as errors. Don't use EffectiveSeverity() here. void Message::Emit(llvm::raw_ostream &o, const AllCookedSources &allCooked, bool echoSourceLine, const common::LanguageFeatureControl *hintFlagPtr) const { From da3cddb3dc58580fb7d0ed12bb9cb608b8397dd1 Mon Sep 17 00:00:00 2001 From: Andre Kuhlenschmidt Date: Wed, 16 Jul 2025 11:27:13 -0700 Subject: [PATCH 6/6] clarify code --- flang/lib/Parser/message.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/flang/lib/Parser/message.cpp b/flang/lib/Parser/message.cpp index 064494de9b478..2a8101dd0b810 100644 --- a/flang/lib/Parser/message.cpp +++ b/flang/lib/Parser/message.cpp @@ -492,12 +492,17 @@ void Messages::AttachTo(Message &msg, std::optional severity) { } bool Messages::AnyFatalError(bool warningsAreErrors) const { + // Short-circuit in the most common case. if (messages_.empty()) { return false; } + // If warnings are errors and there are warnings or errors, this is fatal. + // This preserves the compiler's current behavior of treating any non-fatal + // message as a warning. We may want to refine this in the future. if (warningsAreErrors) { return true; } + // Otherwise, check the message buffer for fatal errors. for (const auto &msg : messages_) { if (msg.IsFatal()) { return true;