[PATCH] D85851: [flang][directives] Use TableGen to generate clause unparsing

[Kiran Chandramohan via Phabricator via llvm-commits]

kiranchandramohan accepted this revision.
kiranchandramohan added a comment.
This revision is now accepted and ready to land.

LGTM. I have mentioned a few nits. You also have some clang-tidy warnings to fix.

BTW, the tests do not seem to be using the new fields introduced in this patch. Would it make sense to use them in the test?

  bit valueIsList = 0;
  string defaultValue = "";



================
Comment at: llvm/utils/TableGen/DirectiveEmitter.cpp:183
 
+  bool valueIsList() const { return Def->getValueAsBit("valueIsList"); }
+
----------------
Nit: Should the name be isValueList to remain consistent with other boolean functions?


================
Comment at: llvm/utils/TableGen/DirectiveEmitter.cpp:550-559
+      OS << "WRAPPER_CLASS(" << Clause.getFormattedParserClassName() << ", ";
+      if (Clause.isValueOptional() && Clause.valueIsList()) {
+        OS << "std::optional<std::list<" << Clause.getFlangClassValue()
+           << ">>);\n";
+      } else if (Clause.isValueOptional()) {
+        OS << "std::optional<" << Clause.getFlangClassValue() << ">);\n";
+      } else if (Clause.valueIsList()) {
----------------
Nit: Was just checking whether this code can be improved.
At least printing of  ");\n" can be common.


================
Comment at: llvm/utils/TableGen/DirectiveEmitter.cpp:603
+// Generate Unparse functions for clauses classes in the Flang parse-tree
+// If the clause as a non-generic class, no entry is generated.
+void GenerateFlangClauseUnparse(const std::vector<Record *> &Clauses,
----------------
Nit: as -> is


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85851/new/

https://reviews.llvm.org/D85851