[flang] remove sequences of duplicate messages #161916

akuhlens · 2025-10-03T22:00:38Z

Fixes bug exposed by #161915 by keeping a cache of messages printed at a given location.

llvmbot · 2025-10-03T22:04:38Z

@llvm/pr-subscribers-flang-parser

Author: Andre Kuhlenschmidt (akuhlens)

Changes

Fixes bug exposed by #161915 by keeping a cache of messages printed at a given location.

Full diff: https://github.com/llvm/llvm-project/pull/161916.diff

6 Files Affected:

(modified) flang/include/flang/Parser/message.h (+1-1)
(modified) flang/lib/Evaluate/check-expression.cpp (+11-6)
(modified) flang/lib/Parser/message.cpp (+20-4)
(modified) flang/test/Semantics/associated.f90 (-2)
(modified) flang/test/Semantics/intrinsics03.f90 (+1-1)
(modified) flang/test/Semantics/intrinsics04.f90 (+1-1)

diff --git a/flang/include/flang/Parser/message.h b/flang/include/flang/Parser/message.h
index 224263e4be860..7c639eff1eeef 100644
--- a/flang/include/flang/Parser/message.h
+++ b/flang/include/flang/Parser/message.h
@@ -307,9 +307,9 @@ class Message : public common::ReferenceCounted<Message> {
   bool Merge(const Message &);
   bool operator==(const Message &that) const;
   bool operator!=(const Message &that) const { return !(*this == that); }
+  bool AtSameLocation(const Message &) const;
 
 private:
-  bool AtSameLocation(const Message &) const;
   std::variant<ProvenanceRange, CharBlock> location_;
   std::variant<MessageFixedText, MessageFormattedText, MessageExpectedText>
       text_;
diff --git a/flang/lib/Evaluate/check-expression.cpp b/flang/lib/Evaluate/check-expression.cpp
index 8931cbe485ac2..012b44e7d3747 100644
--- a/flang/lib/Evaluate/check-expression.cpp
+++ b/flang/lib/Evaluate/check-expression.cpp
@@ -135,16 +135,21 @@ bool IsConstantExprHelper<INVARIANT>::operator()(
     } else if (proc.IsPure()) {
       std::size_t j{0};
       for (const auto &arg : call.arguments()) {
-        if (const auto *dataDummy{j < proc.dummyArguments.size()
-                    ? std::get_if<characteristics::DummyDataObject>(
-                          &proc.dummyArguments[j].u)
-                    : nullptr};
-            dataDummy &&
+        const auto *dataDummy{j < proc.dummyArguments.size()
+                ? std::get_if<characteristics::DummyDataObject>(
+                      &proc.dummyArguments[j].u)
+                : nullptr};
+        if (dataDummy &&
             dataDummy->attrs.test(
                 characteristics::DummyDataObject::Attr::OnlyIntrinsicInquiry)) {
           // The value of the argument doesn't matter
         } else if (!arg) {
-          return false;
+          // Missing optional arguments are constant
+          if (!(dataDummy &&
+                  dataDummy->attrs.test(
+                      characteristics::DummyDataObject::Attr::Optional))) {
+            return false;
+          }
         } else if (const auto *expr{arg->UnwrapExpr()};
             !expr || !(*this)(*expr)) {
           return false;
diff --git a/flang/lib/Parser/message.cpp b/flang/lib/Parser/message.cpp
index 2c4f930c0b088..0e06711c5b71b 100644
--- a/flang/lib/Parser/message.cpp
+++ b/flang/lib/Parser/message.cpp
@@ -477,15 +477,31 @@ void Messages::Emit(llvm::raw_ostream &o, const AllCookedSources &allCooked,
   }
   std::stable_sort(sorted.begin(), sorted.end(),
       [](const Message *x, const Message *y) { return x->SortBefore(*y); });
-  const Message *lastMsg{nullptr};
+  std::vector<const Message *> msgsWithLastLocation;
   std::size_t errorsEmitted{0};
   for (const Message *msg : sorted) {
-    if (lastMsg && *msg == *lastMsg) {
-      // Don't emit two identical messages for the same location
+    bool shouldSkipMsg = false;
+    // Don't emit two identical messages for the same location
+    // At the same location messages are sorted by the order they were
+    // added to the list, which is a decent proxy for the causality
+    // of the messages.
+    if (!msgsWithLastLocation.empty()) {
+      if (msgsWithLastLocation[0]->AtSameLocation(*msg)) {
+        for (const Message *msgAtThisLocation : msgsWithLastLocation) {
+          if (*msg == *msgAtThisLocation) {
+            shouldSkipMsg = true; // continue loop over sorted messages
+            break;
+          }
+        }
+      } else {
+        msgsWithLastLocation.clear();
+      }
+    }
+    if (shouldSkipMsg) {
       continue;
     }
+    msgsWithLastLocation.push_back(msg);
     msg->Emit(o, allCooked, echoSourceLines, hintFlagPtr);
-    lastMsg = msg;
     if (warningsAreErrors || msg->IsFatal()) {
       ++errorsEmitted;
     }
diff --git a/flang/test/Semantics/associated.f90 b/flang/test/Semantics/associated.f90
index 7cb6c240db226..731f2d828995f 100644
--- a/flang/test/Semantics/associated.f90
+++ b/flang/test/Semantics/associated.f90
@@ -253,8 +253,6 @@ subroutine test(assumedRank)
     lvar = associated(intPointerVar1, targetIntCoarray[1])
     !ERROR: 'neverdeclared' is not a procedure
     !ERROR: Could not characterize intrinsic function actual argument 'badpointer'
-    !ERROR: 'neverdeclared' is not a procedure
-    !ERROR: Could not characterize intrinsic function actual argument 'badpointer'
     lvar = associated(badPointer)
   end subroutine test
 end subroutine assoc
diff --git a/flang/test/Semantics/intrinsics03.f90 b/flang/test/Semantics/intrinsics03.f90
index a5b13b655cf41..1a4269868a3d4 100644
--- a/flang/test/Semantics/intrinsics03.f90
+++ b/flang/test/Semantics/intrinsics03.f90
@@ -129,6 +129,6 @@ subroutine ichar_tests()
   !Without -Wportability, the warning isn't emitted and the parameter is constant.
   integer, parameter :: a2 = ichar('B ')
   !ERROR: Character in intrinsic function ichar must have length one
-  !ERROR: Must be a constant value
+  !ERROR: Value of named constant 'a3' (ichar("")) cannot be computed as a constant value
   integer, parameter :: a3 = ichar('')
 end subroutine
diff --git a/flang/test/Semantics/intrinsics04.f90 b/flang/test/Semantics/intrinsics04.f90
index abb8fe321a572..e733067237d17 100644
--- a/flang/test/Semantics/intrinsics04.f90
+++ b/flang/test/Semantics/intrinsics04.f90
@@ -29,6 +29,6 @@ subroutine ichar_tests()
   !WARNING: Character in intrinsic function ichar should have length one [-Wportability]
   integer, parameter :: a2 = ichar('B ')
   !ERROR: Character in intrinsic function ichar must have length one
-  !ERROR: Must be a constant value
+  !ERROR: Value of named constant 'a3' (ichar("")) cannot be computed as a constant value
   integer, parameter :: a3 = ichar('')
 end subroutine

llvmbot · 2025-10-03T22:04:39Z

@llvm/pr-subscribers-flang-semantics

Author: Andre Kuhlenschmidt (akuhlens)

Changes

Fixes bug exposed by #161915 by keeping a cache of messages printed at a given location.

Full diff: https://github.com/llvm/llvm-project/pull/161916.diff

6 Files Affected:

(modified) flang/include/flang/Parser/message.h (+1-1)
(modified) flang/lib/Evaluate/check-expression.cpp (+11-6)
(modified) flang/lib/Parser/message.cpp (+20-4)
(modified) flang/test/Semantics/associated.f90 (-2)
(modified) flang/test/Semantics/intrinsics03.f90 (+1-1)
(modified) flang/test/Semantics/intrinsics04.f90 (+1-1)

diff --git a/flang/include/flang/Parser/message.h b/flang/include/flang/Parser/message.h
index 224263e4be860..7c639eff1eeef 100644
--- a/flang/include/flang/Parser/message.h
+++ b/flang/include/flang/Parser/message.h
@@ -307,9 +307,9 @@ class Message : public common::ReferenceCounted<Message> {
   bool Merge(const Message &);
   bool operator==(const Message &that) const;
   bool operator!=(const Message &that) const { return !(*this == that); }
+  bool AtSameLocation(const Message &) const;
 
 private:
-  bool AtSameLocation(const Message &) const;
   std::variant<ProvenanceRange, CharBlock> location_;
   std::variant<MessageFixedText, MessageFormattedText, MessageExpectedText>
       text_;
diff --git a/flang/lib/Evaluate/check-expression.cpp b/flang/lib/Evaluate/check-expression.cpp
index 8931cbe485ac2..012b44e7d3747 100644
--- a/flang/lib/Evaluate/check-expression.cpp
+++ b/flang/lib/Evaluate/check-expression.cpp
@@ -135,16 +135,21 @@ bool IsConstantExprHelper<INVARIANT>::operator()(
     } else if (proc.IsPure()) {
       std::size_t j{0};
       for (const auto &arg : call.arguments()) {
-        if (const auto *dataDummy{j < proc.dummyArguments.size()
-                    ? std::get_if<characteristics::DummyDataObject>(
-                          &proc.dummyArguments[j].u)
-                    : nullptr};
-            dataDummy &&
+        const auto *dataDummy{j < proc.dummyArguments.size()
+                ? std::get_if<characteristics::DummyDataObject>(
+                      &proc.dummyArguments[j].u)
+                : nullptr};
+        if (dataDummy &&
             dataDummy->attrs.test(
                 characteristics::DummyDataObject::Attr::OnlyIntrinsicInquiry)) {
           // The value of the argument doesn't matter
         } else if (!arg) {
-          return false;
+          // Missing optional arguments are constant
+          if (!(dataDummy &&
+                  dataDummy->attrs.test(
+                      characteristics::DummyDataObject::Attr::Optional))) {
+            return false;
+          }
         } else if (const auto *expr{arg->UnwrapExpr()};
             !expr || !(*this)(*expr)) {
           return false;
diff --git a/flang/lib/Parser/message.cpp b/flang/lib/Parser/message.cpp
index 2c4f930c0b088..0e06711c5b71b 100644
--- a/flang/lib/Parser/message.cpp
+++ b/flang/lib/Parser/message.cpp
@@ -477,15 +477,31 @@ void Messages::Emit(llvm::raw_ostream &o, const AllCookedSources &allCooked,
   }
   std::stable_sort(sorted.begin(), sorted.end(),
       [](const Message *x, const Message *y) { return x->SortBefore(*y); });
-  const Message *lastMsg{nullptr};
+  std::vector<const Message *> msgsWithLastLocation;
   std::size_t errorsEmitted{0};
   for (const Message *msg : sorted) {
-    if (lastMsg && *msg == *lastMsg) {
-      // Don't emit two identical messages for the same location
+    bool shouldSkipMsg = false;
+    // Don't emit two identical messages for the same location
+    // At the same location messages are sorted by the order they were
+    // added to the list, which is a decent proxy for the causality
+    // of the messages.
+    if (!msgsWithLastLocation.empty()) {
+      if (msgsWithLastLocation[0]->AtSameLocation(*msg)) {
+        for (const Message *msgAtThisLocation : msgsWithLastLocation) {
+          if (*msg == *msgAtThisLocation) {
+            shouldSkipMsg = true; // continue loop over sorted messages
+            break;
+          }
+        }
+      } else {
+        msgsWithLastLocation.clear();
+      }
+    }
+    if (shouldSkipMsg) {
       continue;
     }
+    msgsWithLastLocation.push_back(msg);
     msg->Emit(o, allCooked, echoSourceLines, hintFlagPtr);
-    lastMsg = msg;
     if (warningsAreErrors || msg->IsFatal()) {
       ++errorsEmitted;
     }
diff --git a/flang/test/Semantics/associated.f90 b/flang/test/Semantics/associated.f90
index 7cb6c240db226..731f2d828995f 100644
--- a/flang/test/Semantics/associated.f90
+++ b/flang/test/Semantics/associated.f90
@@ -253,8 +253,6 @@ subroutine test(assumedRank)
     lvar = associated(intPointerVar1, targetIntCoarray[1])
     !ERROR: 'neverdeclared' is not a procedure
     !ERROR: Could not characterize intrinsic function actual argument 'badpointer'
-    !ERROR: 'neverdeclared' is not a procedure
-    !ERROR: Could not characterize intrinsic function actual argument 'badpointer'
     lvar = associated(badPointer)
   end subroutine test
 end subroutine assoc
diff --git a/flang/test/Semantics/intrinsics03.f90 b/flang/test/Semantics/intrinsics03.f90
index a5b13b655cf41..1a4269868a3d4 100644
--- a/flang/test/Semantics/intrinsics03.f90
+++ b/flang/test/Semantics/intrinsics03.f90
@@ -129,6 +129,6 @@ subroutine ichar_tests()
   !Without -Wportability, the warning isn't emitted and the parameter is constant.
   integer, parameter :: a2 = ichar('B ')
   !ERROR: Character in intrinsic function ichar must have length one
-  !ERROR: Must be a constant value
+  !ERROR: Value of named constant 'a3' (ichar("")) cannot be computed as a constant value
   integer, parameter :: a3 = ichar('')
 end subroutine
diff --git a/flang/test/Semantics/intrinsics04.f90 b/flang/test/Semantics/intrinsics04.f90
index abb8fe321a572..e733067237d17 100644
--- a/flang/test/Semantics/intrinsics04.f90
+++ b/flang/test/Semantics/intrinsics04.f90
@@ -29,6 +29,6 @@ subroutine ichar_tests()
   !WARNING: Character in intrinsic function ichar should have length one [-Wportability]
   integer, parameter :: a2 = ichar('B ')
   !ERROR: Character in intrinsic function ichar must have length one
-  !ERROR: Must be a constant value
+  !ERROR: Value of named constant 'a3' (ichar("")) cannot be computed as a constant value
   integer, parameter :: a3 = ichar('')
 end subroutine

flang/lib/Evaluate/check-expression.cpp

flang/lib/Parser/message.cpp

…ument (#161915) fixes #161694 Exposes that some sequences of duplicate messages are being printed, which is fixed in #161916 .

…ptional argument (#161915) fixes llvm/llvm-project#161694 Exposes that some sequences of duplicate messages are being printed, which is fixed in llvm/llvm-project#161916 .

akuhlens requested review from klausler and eugeneepshteyn October 3, 2025 22:00

akuhlens mentioned this pull request Oct 3, 2025

[flang][semantics] fix IsConstantExpr for intrinsic with optional argument #161915

Merged

llvmbot added flang Flang issues not falling into any other category flang:semantics flang:parser labels Oct 3, 2025

klausler reviewed Oct 3, 2025

View reviewed changes

flang/lib/Evaluate/check-expression.cpp Outdated Show resolved Hide resolved

flang/lib/Parser/message.cpp Outdated Show resolved Hide resolved

eugeneepshteyn reviewed Oct 3, 2025

View reviewed changes

flang/lib/Parser/message.cpp Show resolved Hide resolved

akuhlens force-pushed the andre/remove-duplicate-messages branch from 7dba404 to 49d77cc Compare October 3, 2025 22:28

akuhlens requested review from klausler and eugeneepshteyn October 3, 2025 22:30

klausler approved these changes Oct 3, 2025

View reviewed changes

akuhlens added 3 commits October 3, 2025 16:14

initial commit

3e11d68

don't used brace initialization

22e8f06

clean up comment

7bb937e

akuhlens force-pushed the andre/remove-duplicate-messages branch from dd1fb1d to 7bb937e Compare October 3, 2025 23:15

akuhlens merged commit b1e29ec into llvm:main Oct 4, 2025
9 checks passed

akuhlens added a commit that referenced this pull request Oct 4, 2025

[flang][semantics] fix IsConstantExpr for intrinsic with optional arg…

823094e

…ument (#161915) fixes #161694 Exposes that some sequences of duplicate messages are being printed, which is fixed in #161916 .

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[flang] remove sequences of duplicate messages #161916

[flang] remove sequences of duplicate messages #161916

Uh oh!

akuhlens commented Oct 3, 2025

Uh oh!

llvmbot commented Oct 3, 2025

Uh oh!

llvmbot commented Oct 3, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

[flang] remove sequences of duplicate messages #161916

[flang] remove sequences of duplicate messages #161916

Uh oh!

Conversation

akuhlens commented Oct 3, 2025

Uh oh!

llvmbot commented Oct 3, 2025

Uh oh!

llvmbot commented Oct 3, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!