[flang] Move DumpEvaluateExpr from Lower to Semantics #128723

kparzysz · 2025-02-25T15:06:30Z

Since evaluate::Expr can show up in the parse tree in the semantic analysis step, make it possible to dump its structure in the Semantics module.

The Lower module depends on Semantics, so the code is still accessible in it.

Since evaluate::Expr can show up in the parse tree in the semantic analysis step, make it possible to dump its structure in the Semantics module. The Lower module depends on Semantics, so the code is still accessible in it.

llvmbot · 2025-02-25T15:07:18Z

@llvm/pr-subscribers-flang-semantics

@llvm/pr-subscribers-flang-fir-hlfir

Author: Krzysztof Parzyszek (kparzysz)

Changes

Since evaluate::Expr can show up in the parse tree in the semantic analysis step, make it possible to dump its structure in the Semantics module.

The Lower module depends on Semantics, so the code is still accessible in it.

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

5 Files Affected:

(renamed) flang/include/flang/Semantics/dump-expr.h (+6-7)
(modified) flang/lib/Lower/CMakeLists.txt (-1)
(modified) flang/lib/Lower/ConvertExpr.cpp (+3-3)
(modified) flang/lib/Semantics/CMakeLists.txt (+1)
(renamed) flang/lib/Semantics/dump-expr.cpp (+41-40)

diff --git a/flang/include/flang/Lower/DumpEvaluateExpr.h b/flang/include/flang/Semantics/dump-expr.h
similarity index 95%
rename from flang/include/flang/Lower/DumpEvaluateExpr.h
rename to flang/include/flang/Semantics/dump-expr.h
index 88f53e96a81c2..d51cb6539d29b 100644
--- a/flang/include/flang/Lower/DumpEvaluateExpr.h
+++ b/flang/include/flang/Semantics/dump-expr.h
@@ -1,4 +1,4 @@
-//===-- Lower/DumpEvaluateExpr.h --------------------------------*- C++ -*-===//
+//===-- Semantics/DumpEvaluateExpr.h ----------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
@@ -6,15 +6,14 @@
 //
 //===----------------------------------------------------------------------===//
 
-#ifndef FORTRAN_LOWER_DUMPEVALUATEEXPR_H
-#define FORTRAN_LOWER_DUMPEVALUATEEXPR_H
+#ifndef FORTRAN_SEMANTICS_DUMPEVALUATEEXPR_H
+#define FORTRAN_SEMANTICS_DUMPEVALUATEEXPR_H
 
 #include "flang/Evaluate/tools.h"
-#include "flang/Lower/Support/Utils.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Twine.h"
 
-namespace Fortran::lower {
+namespace Fortran::semantics {
 
 /// Class to dump Fortran::evaluate::Expr trees out in a user readable way.
 ///
@@ -207,6 +206,6 @@ LLVM_DUMP_METHOD void dumpEvExpr(
     const Fortran::evaluate::Designator<
         Fortran::evaluate::Type<Fortran::common::TypeCategory::Integer, 4>> &x);
 
-} // namespace Fortran::lower
+} // namespace Fortran::semantics
 
-#endif // FORTRAN_LOWER_DUMPEVALUATEEXPR_H
+#endif // FORTRAN_SEMANTICS_DUMPEVALUATEEXPR_H
diff --git a/flang/lib/Lower/CMakeLists.txt b/flang/lib/Lower/CMakeLists.txt
index 87dc2a052796a..0bd9a47cd040f 100644
--- a/flang/lib/Lower/CMakeLists.txt
+++ b/flang/lib/Lower/CMakeLists.txt
@@ -16,7 +16,6 @@ add_flang_library(FortranLower
   ConvertType.cpp
   ConvertVariable.cpp
   CustomIntrinsicCall.cpp
-  DumpEvaluateExpr.cpp
   HlfirIntrinsics.cpp
   HostAssociations.cpp
   IO.cpp
diff --git a/flang/lib/Lower/ConvertExpr.cpp b/flang/lib/Lower/ConvertExpr.cpp
index b33baf90582b8..41b9ab52c52b0 100644
--- a/flang/lib/Lower/ConvertExpr.cpp
+++ b/flang/lib/Lower/ConvertExpr.cpp
@@ -27,7 +27,6 @@
 #include "flang/Lower/ConvertType.h"
 #include "flang/Lower/ConvertVariable.h"
 #include "flang/Lower/CustomIntrinsicCall.h"
-#include "flang/Lower/DumpEvaluateExpr.h"
 #include "flang/Lower/Mangler.h"
 #include "flang/Lower/Runtime.h"
 #include "flang/Lower/Support/Utils.h"
@@ -47,6 +46,7 @@
 #include "flang/Optimizer/Dialect/FIROpsSupport.h"
 #include "flang/Optimizer/Support/FatalError.h"
 #include "flang/Runtime/support.h"
+#include "flang/Semantics/dump-expr.h"
 #include "flang/Semantics/expression.h"
 #include "flang/Semantics/symbol.h"
 #include "flang/Semantics/tools.h"
@@ -3925,7 +3925,7 @@ class ArrayExprLowering {
   /// determine the actual number of iterations when slicing triples are
   /// present. Lower these expressions here.
   bool determineShapeWithSlice(const Fortran::lower::SomeExpr &lhs) {
-    LLVM_DEBUG(Fortran::lower::DumpEvaluateExpr::dump(
+    LLVM_DEBUG(Fortran::semantics::DumpEvaluateExpr::dump(
         llvm::dbgs() << "determine shape of:\n", lhs));
     // FIXME: We may not want to use ExtractDataRef here since it doesn't deal
     // with substrings, etc.
@@ -5073,7 +5073,7 @@ class ArrayExprLowering {
 
   template <typename A>
   CC genarr(const Fortran::evaluate::Expr<A> &x) {
-    LLVM_DEBUG(Fortran::lower::DumpEvaluateExpr::dump(llvm::dbgs(), x));
+    LLVM_DEBUG(Fortran::semantics::DumpEvaluateExpr::dump(llvm::dbgs(), x));
     if (isArray(x) || (explicitSpaceIsActive() && isLeftHandSide()) ||
         isElementalProcWithArrayArgs(x))
       return Fortran::common::visit([&](const auto &e) { return genarr(e); },
diff --git a/flang/lib/Semantics/CMakeLists.txt b/flang/lib/Semantics/CMakeLists.txt
index 00108dde49dbd..93bf0c7c5facd 100644
--- a/flang/lib/Semantics/CMakeLists.txt
+++ b/flang/lib/Semantics/CMakeLists.txt
@@ -29,6 +29,7 @@ add_flang_library(FortranSemantics
   compute-offsets.cpp
   data-to-inits.cpp
   definable.cpp
+  dump-expr.cpp
   expression.cpp
   mod-file.cpp
   openmp-modifiers.cpp
diff --git a/flang/lib/Lower/DumpEvaluateExpr.cpp b/flang/lib/Semantics/dump-expr.cpp
similarity index 70%
rename from flang/lib/Lower/DumpEvaluateExpr.cpp
rename to flang/lib/Semantics/dump-expr.cpp
index 9273e94d702f8..f0b0ecabf950a 100644
--- a/flang/lib/Lower/DumpEvaluateExpr.cpp
+++ b/flang/lib/Semantics/dump-expr.cpp
@@ -1,4 +1,4 @@
-//===-- Lower/DumpEvaluateExpr.cpp ----------------------------------------===//
+//===-- Semantics/DumpEvaluateExpr.cpp ------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
@@ -6,19 +6,20 @@
 //
 //===----------------------------------------------------------------------===//
 
-#include "flang/Lower/DumpEvaluateExpr.h"
+#include "flang/Semantics/dump-expr.h"
 #include <iostream>
 
 static constexpr char whiteSpacePadding[] =
     ">>                                               ";
 static constexpr auto whiteSize = sizeof(whiteSpacePadding) - 1;
 
-inline const char *Fortran::lower::DumpEvaluateExpr::getIndentString() const {
+inline const char *
+Fortran::semantics::DumpEvaluateExpr::getIndentString() const {
   auto count = (level * 2 >= whiteSize) ? whiteSize : level * 2;
   return whiteSpacePadding + whiteSize - count;
 }
 
-void Fortran::lower::DumpEvaluateExpr::show(
+void Fortran::semantics::DumpEvaluateExpr::show(
     const Fortran::evaluate::CoarrayRef &x) {
   indent("coarray ref");
   show(x.base());
@@ -29,20 +30,20 @@ void Fortran::lower::DumpEvaluateExpr::show(
   outdent();
 }
 
-void Fortran::lower::DumpEvaluateExpr::show(
+void Fortran::semantics::DumpEvaluateExpr::show(
     const Fortran::evaluate::BOZLiteralConstant &) {
   print("BOZ literal constant");
 }
 
-void Fortran::lower::DumpEvaluateExpr::show(
+void Fortran::semantics::DumpEvaluateExpr::show(
     const Fortran::evaluate::NullPointer &) {
   print("null pointer");
 }
 
-void Fortran::lower::DumpEvaluateExpr::show(
+void Fortran::semantics::DumpEvaluateExpr::show(
     const Fortran::semantics::Symbol &symbol) {
   const auto &ultimate{symbol.GetUltimate()};
-  print("symbol: "s + std::string(toStringRef(symbol.name())));
+  print("symbol: "s + symbol.name().ToString());
   if (const auto *assoc =
           ultimate.detailsIf<Fortran::semantics::AssocEntityDetails>()) {
     indent("assoc details");
@@ -51,23 +52,23 @@ void Fortran::lower::DumpEvaluateExpr::show(
   }
 }
 
-void Fortran::lower::DumpEvaluateExpr::show(
+void Fortran::semantics::DumpEvaluateExpr::show(
     const Fortran::evaluate::StaticDataObject &) {
   print("static data object");
 }
 
-void Fortran::lower::DumpEvaluateExpr::show(
+void Fortran::semantics::DumpEvaluateExpr::show(
     const Fortran::evaluate::ImpliedDoIndex &) {
   print("implied do index");
 }
 
-void Fortran::lower::DumpEvaluateExpr::show(
+void Fortran::semantics::DumpEvaluateExpr::show(
     const Fortran::evaluate::BaseObject &x) {
   indent("base object");
   show(x.u);
   outdent();
 }
-void Fortran::lower::DumpEvaluateExpr::show(
+void Fortran::semantics::DumpEvaluateExpr::show(
     const Fortran::evaluate::Component &x) {
   indent("component");
   show(x.base());
@@ -75,7 +76,7 @@ void Fortran::lower::DumpEvaluateExpr::show(
   outdent();
 }
 
-void Fortran::lower::DumpEvaluateExpr::show(
+void Fortran::semantics::DumpEvaluateExpr::show(
     const Fortran::evaluate::NamedEntity &x) {
   indent("named entity");
   if (const auto *component = x.UnwrapComponent())
@@ -85,14 +86,14 @@ void Fortran::lower::DumpEvaluateExpr::show(
   outdent();
 }
 
-void Fortran::lower::DumpEvaluateExpr::show(
+void Fortran::semantics::DumpEvaluateExpr::show(
     const Fortran::evaluate::TypeParamInquiry &x) {
   indent("type inquiry");
   show(x.base());
   outdent();
 }
 
-void Fortran::lower::DumpEvaluateExpr::show(
+void Fortran::semantics::DumpEvaluateExpr::show(
     const Fortran::evaluate::Triplet &x) {
   indent("triplet");
   show(x.lower());
@@ -101,14 +102,14 @@ void Fortran::lower::DumpEvaluateExpr::show(
   outdent();
 }
 
-void Fortran::lower::DumpEvaluateExpr::show(
+void Fortran::semantics::DumpEvaluateExpr::show(
     const Fortran::evaluate::Subscript &x) {
   indent("subscript");
   show(x.u);
   outdent();
 }
 
-void Fortran::lower::DumpEvaluateExpr::show(
+void Fortran::semantics::DumpEvaluateExpr::show(
     const Fortran::evaluate::ArrayRef &x) {
   indent("array ref");
   show(x.base());
@@ -116,14 +117,14 @@ void Fortran::lower::DumpEvaluateExpr::show(
   outdent();
 }
 
-void Fortran::lower::DumpEvaluateExpr::show(
+void Fortran::semantics::DumpEvaluateExpr::show(
     const Fortran::evaluate::DataRef &x) {
   indent("data ref");
   show(x.u);
   outdent();
 }
 
-void Fortran::lower::DumpEvaluateExpr::show(
+void Fortran::semantics::DumpEvaluateExpr::show(
     const Fortran::evaluate::Substring &x) {
   indent("substring");
   show(x.parent());
@@ -132,20 +133,20 @@ void Fortran::lower::DumpEvaluateExpr::show(
   outdent();
 }
 
-void Fortran::lower::DumpEvaluateExpr::show(
+void Fortran::semantics::DumpEvaluateExpr::show(
     const Fortran::semantics::ParamValue &x) {
   indent("param value");
   show(x.GetExplicit());
   outdent();
 }
 
-void Fortran::lower::DumpEvaluateExpr::show(
+void Fortran::semantics::DumpEvaluateExpr::show(
     const Fortran::semantics::DerivedTypeSpec::ParameterMapType::value_type
         &x) {
   show(x.second);
 }
 
-void Fortran::lower::DumpEvaluateExpr::show(
+void Fortran::semantics::DumpEvaluateExpr::show(
     const Fortran::semantics::DerivedTypeSpec &x) {
   indent("derived type spec");
   for (auto &v : x.parameters())
@@ -153,12 +154,12 @@ void Fortran::lower::DumpEvaluateExpr::show(
   outdent();
 }
 
-void Fortran::lower::DumpEvaluateExpr::show(
+void Fortran::semantics::DumpEvaluateExpr::show(
     const Fortran::evaluate::StructureConstructorValues::value_type &x) {
   show(x.second);
 }
 
-void Fortran::lower::DumpEvaluateExpr::show(
+void Fortran::semantics::DumpEvaluateExpr::show(
     const Fortran::evaluate::StructureConstructor &x) {
   indent("structure constructor");
   show(x.derivedTypeSpec());
@@ -167,21 +168,21 @@ void Fortran::lower::DumpEvaluateExpr::show(
   outdent();
 }
 
-void Fortran::lower::DumpEvaluateExpr::show(
+void Fortran::semantics::DumpEvaluateExpr::show(
     const Fortran::evaluate::Relational<Fortran::evaluate::SomeType> &x) {
   indent("expr some type");
   show(x.u);
   outdent();
 }
 
-void Fortran::lower::DumpEvaluateExpr::show(
+void Fortran::semantics::DumpEvaluateExpr::show(
     const Fortran::evaluate::ComplexPart &x) {
   indent("complex part");
   show(x.complex());
   outdent();
 }
 
-void Fortran::lower::DumpEvaluateExpr::show(
+void Fortran::semantics::DumpEvaluateExpr::show(
     const Fortran::evaluate::ActualArgument &x) {
   indent("actual argument");
   if (const auto *symbol = x.GetAssumedTypeDummy())
@@ -191,7 +192,7 @@ void Fortran::lower::DumpEvaluateExpr::show(
   outdent();
 }
 
-void Fortran::lower::DumpEvaluateExpr::show(
+void Fortran::semantics::DumpEvaluateExpr::show(
     const Fortran::evaluate::ProcedureDesignator &x) {
   indent("procedure designator");
   if (const auto *component = x.GetComponent())
@@ -203,28 +204,28 @@ void Fortran::lower::DumpEvaluateExpr::show(
   outdent();
 }
 
-void Fortran::lower::DumpEvaluateExpr::show(
+void Fortran::semantics::DumpEvaluateExpr::show(
     const Fortran::evaluate::SpecificIntrinsic &) {
   print("specific intrinsic");
 }
 
-void Fortran::lower::DumpEvaluateExpr::show(
+void Fortran::semantics::DumpEvaluateExpr::show(
     const Fortran::evaluate::DescriptorInquiry &x) {
   indent("descriptor inquiry");
   show(x.base());
   outdent();
 }
 
-void Fortran::lower::DumpEvaluateExpr::print(llvm::Twine twine) {
+void Fortran::semantics::DumpEvaluateExpr::print(llvm::Twine twine) {
   outs << getIndentString() << twine << '\n';
 }
 
-void Fortran::lower::DumpEvaluateExpr::indent(llvm::StringRef s) {
+void Fortran::semantics::DumpEvaluateExpr::indent(llvm::StringRef s) {
   print(s + " {");
   level++;
 }
 
-void Fortran::lower::DumpEvaluateExpr::outdent() {
+void Fortran::semantics::DumpEvaluateExpr::outdent() {
   if (level)
     level--;
   print("}");
@@ -234,37 +235,37 @@ void Fortran::lower::DumpEvaluateExpr::outdent() {
 // Boilerplate entry points that the debugger can find.
 //===----------------------------------------------------------------------===//
 
-void Fortran::lower::dumpEvExpr(const Fortran::semantics::SomeExpr &x) {
+void Fortran::semantics::dumpEvExpr(const Fortran::semantics::SomeExpr &x) {
   DumpEvaluateExpr::dump(x);
 }
 
-void Fortran::lower::dumpEvExpr(
+void Fortran::semantics::dumpEvExpr(
     const Fortran::evaluate::Expr<
         Fortran::evaluate::Type<Fortran::common::TypeCategory::Integer, 4>>
         &x) {
   DumpEvaluateExpr::dump(x);
 }
 
-void Fortran::lower::dumpEvExpr(
+void Fortran::semantics::dumpEvExpr(
     const Fortran::evaluate::Expr<
         Fortran::evaluate::Type<Fortran::common::TypeCategory::Integer, 8>>
         &x) {
   DumpEvaluateExpr::dump(x);
 }
 
-void Fortran::lower::dumpEvExpr(const Fortran::evaluate::ArrayRef &x) {
+void Fortran::semantics::dumpEvExpr(const Fortran::evaluate::ArrayRef &x) {
   DumpEvaluateExpr::dump(x);
 }
 
-void Fortran::lower::dumpEvExpr(const Fortran::evaluate::DataRef &x) {
+void Fortran::semantics::dumpEvExpr(const Fortran::evaluate::DataRef &x) {
   DumpEvaluateExpr::dump(x);
 }
 
-void Fortran::lower::dumpEvExpr(const Fortran::evaluate::Substring &x) {
+void Fortran::semantics::dumpEvExpr(const Fortran::evaluate::Substring &x) {
   DumpEvaluateExpr::dump(x);
 }
 
-void Fortran::lower::dumpEvExpr(
+void Fortran::semantics::dumpEvExpr(
     const Fortran::evaluate::Designator<
         Fortran::evaluate::Type<Fortran::common::TypeCategory::Integer, 4>>
         &x) {

github-actions · 2025-02-25T15:11:52Z

✅ With the latest revision this PR passed the C/C++ code formatter.

klausler

Would this not make more sense in Evaluate rather than Semantics?

Both Evaluate and Semantics use a stricter set of coding style conventions than lowering does. Those conventions should be adopted by this code if it is moved into one of those directories.

kparzysz · 2025-02-25T18:19:49Z

I actually tried to put it in Evaluate, but some of the show functions take members of semantics as arguments, e.g. Fortran::semantics::SymbolRef or Fortran::semantics::ParamValue. Do you have any suggestions regarding those (there are a few more than these two)?

The formatting changes were demanded by the format checker in the pre-merge builder. Initially, the build failed, then passed with the formatting changes.

kparzysz · 2025-02-26T15:08:12Z

I have changed the code to use brace initialization, plus modified the namespace usage to reduce qualification.

kparzysz · 2025-03-03T20:00:32Z

Ping

klausler · 2025-03-03T20:02:19Z

flang/include/flang/Semantics/dump-expr.h

+  }
+  template <typename A> void Show(const std::vector<A> &x) {
+    Indent("vector");
+    for (const auto &v : x)


braces please

klausler · 2025-03-03T20:02:35Z

flang/include/flang/Semantics/dump-expr.h

+  template <typename T> void Show(const evaluate::Constant<T> &x) {
+    if constexpr (T::category == common::TypeCategory::Derived) {
+      Indent("derived constant");
+      for (const auto &map : x.values())


braces around loop bodies, please

klausler · 2025-03-03T20:03:14Z

flang/lib/Semantics/dump-expr.cpp

+
+void DumpEvaluateExpr::Show(const evaluate::NamedEntity &x) {
+  Indent("named entity");
+  if (const auto *component{x.UnwrapComponent()})


braces around consequent statements, please

Since evaluate::Expr can show up in the parse tree in the semantic analysis step, make it possible to dump its structure in the Semantics module. The Lower module depends on Semantics, so the code is still accessible in it.

[flang] Move DumpEvaluateExpr from Lower to Semantics

36aad22

Since evaluate::Expr can show up in the parse tree in the semantic analysis step, make it possible to dump its structure in the Semantics module. The Lower module depends on Semantics, so the code is still accessible in it.

kparzysz requested review from clementval, jeanPerier and klausler February 25, 2025 15:06

llvmbot added flang Flang issues not falling into any other category flang:fir-hlfir flang:semantics labels Feb 25, 2025

kparzysz added 2 commits February 25, 2025 09:15

Remove unused header

613af06

format

6c0d52e

klausler reviewed Feb 25, 2025

View reviewed changes

kparzysz added 4 commits February 26, 2025 08:03

Use brace initialization

8402e55

Put definitions in namespace to avoid extra qualifications

2529d9a

Fix file name in comment

7913072

Fix name of include guard

fe0ebf9

kparzysz added 2 commits February 26, 2025 18:50

Merge branch 'main' into users/kparzysz/dump-ev-expr

f118265

Change function/member naming

23aad32

klausler reviewed Mar 3, 2025

View reviewed changes

Add braces around if/for bodies

4c6c395

klausler approved these changes Mar 3, 2025

View reviewed changes

kparzysz merged commit 8f971ca into llvm:main Mar 3, 2025
11 checks passed

kparzysz deleted the users/kparzysz/dump-ev-expr branch March 3, 2025 21:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[flang] Move DumpEvaluateExpr from Lower to Semantics #128723

[flang] Move DumpEvaluateExpr from Lower to Semantics #128723

Uh oh!

kparzysz commented Feb 25, 2025

Uh oh!

llvmbot commented Feb 25, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Feb 25, 2025 •

edited

Loading

Uh oh!

klausler left a comment

Uh oh!

kparzysz commented Feb 25, 2025

Uh oh!

kparzysz commented Feb 26, 2025

Uh oh!

kparzysz commented Mar 3, 2025

Uh oh!

klausler Mar 3, 2025

Uh oh!

klausler Mar 3, 2025

Uh oh!

kparzysz Mar 3, 2025

Uh oh!

klausler Mar 3, 2025

Uh oh!

kparzysz Mar 3, 2025

Uh oh!

Uh oh!

Uh oh!

[flang] Move DumpEvaluateExpr from Lower to Semantics #128723

[flang] Move DumpEvaluateExpr from Lower to Semantics #128723

Uh oh!

Conversation

kparzysz commented Feb 25, 2025

Uh oh!

llvmbot commented Feb 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Feb 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

klausler left a comment

Choose a reason for hiding this comment

Uh oh!

kparzysz commented Feb 25, 2025

Uh oh!

kparzysz commented Feb 26, 2025

Uh oh!

kparzysz commented Mar 3, 2025

Uh oh!

klausler Mar 3, 2025

Choose a reason for hiding this comment

Uh oh!

klausler Mar 3, 2025

Choose a reason for hiding this comment

Uh oh!

kparzysz Mar 3, 2025

Choose a reason for hiding this comment

Uh oh!

klausler Mar 3, 2025

Choose a reason for hiding this comment

Uh oh!

kparzysz Mar 3, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

llvmbot commented Feb 25, 2025 •

edited

Loading

github-actions bot commented Feb 25, 2025 •

edited

Loading