Skip to content

Conversation

kparzysz
Copy link
Contributor

Remove 'if std::string' that is covered by another branch of the if-statement.
Add printing of 'bool' and 'int' values, since they have corresponding GetNodeName definitions.

Remove 'if std::string' that is covered by another branch of the
if-statement.
Add printing of 'bool' and 'int' values, since they have corresponding
`GetNodeName` definitions.
@llvmbot llvmbot added flang Flang issues not falling into any other category flang:parser labels Oct 17, 2024
@llvmbot
Copy link
Member

llvmbot commented Oct 17, 2024

@llvm/pr-subscribers-flang-openmp

@llvm/pr-subscribers-flang-parser

Author: Krzysztof Parzyszek (kparzysz)

Changes

Remove 'if std::string' that is covered by another branch of the if-statement.
Add printing of 'bool' and 'int' values, since they have corresponding GetNodeName definitions.


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

1 Files Affected:

  • (modified) flang/include/flang/Parser/dump-parse-tree.h (+4-2)
diff --git a/flang/include/flang/Parser/dump-parse-tree.h b/flang/include/flang/Parser/dump-parse-tree.h
index 5d243b4e5d3e9a..ccbe5475d051e0 100644
--- a/flang/include/flang/Parser/dump-parse-tree.h
+++ b/flang/include/flang/Parser/dump-parse-tree.h
@@ -884,8 +884,10 @@ class ParseTreeDumper {
     } else if constexpr (HasSource<T>::value) {
       return x.source.ToString();
 #endif
-    } else if constexpr (std::is_same_v<T, std::string>) {
-      return x;
+    } else if constexpr (std::is_same_v<T, int>) {
+      return std::to_string(x);
+    } else if constexpr (std::is_same_v<T, bool>) {
+      return x ? "true" : "false";
     } else {
       return "";
     }

@kiranchandramohan
Copy link
Contributor

kiranchandramohan commented Oct 17, 2024

Can we have a test since this patch is non-NFC?

@kparzysz
Copy link
Contributor Author

There aren't really any tests for dumping the parse tree, except for OpenMP. The difference (at least for the int case) would show in a declaration like real, allocatable :: z(:). I can add an OpenMP testcase that has this declaration plus some OMP construct for the sake of being an OpenMP test... What do you think?

@kparzysz
Copy link
Contributor Author

Source:

subroutine foo()
  real, allocatable :: z(:)
end

Tree before:

Program -> ProgramUnit -> SubroutineSubprogram
| SubroutineStmt
| | Name = 'foo'
| SpecificationPart
| | ImplicitPart ->
| | DeclarationConstruct -> SpecificationConstruct -> TypeDeclarationStmt
| | | DeclarationTypeSpec -> IntrinsicTypeSpec -> Real
| | | AttrSpec -> Allocatable
| | | EntityDecl
| | | | Name = 'z'
| | | | ArraySpec -> DeferredShapeSpecList -> int
| ExecutionPart -> Block
| EndSubroutineStmt ->

Tree after (compare the 3rd line from the bottom):

Program -> ProgramUnit -> SubroutineSubprogram
| SubroutineStmt
| | Name = 'foo'
| SpecificationPart
| | ImplicitPart ->
| | DeclarationConstruct -> SpecificationConstruct -> TypeDeclarationStmt
| | | DeclarationTypeSpec -> IntrinsicTypeSpec -> Real
| | | AttrSpec -> Allocatable
| | | EntityDecl
| | | | Name = 'z'
| | | | ArraySpec -> DeferredShapeSpecList -> int = '1'
| ExecutionPart -> Block
| EndSubroutineStmt ->

Copy link
Contributor

@kiranchandramohan kiranchandramohan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can add an OpenMP testcase that has this declaration plus some OMP construct for the sake of being an OpenMP test... What do you think?

If you can add a test that would be great. But no pressure.

@kparzysz kparzysz merged commit 2c93598 into main Oct 17, 2024
9 checks passed
@kparzysz kparzysz deleted the users/kparzysz/spr/a01-dump-parse-tree branch October 17, 2024 17:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flang:openmp flang:parser flang Flang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants