[PATCH] D81034: [flang] add ConvertType to lowering

[Kiran Chandramohan via Phabricator via llvm-commits]

kiranchandramohan added a comment.

A few questions.



================
Comment at: flang/lib/Lower/ConvertType.cpp:263
+  //===--------------------------------------------------------------------===//
+  // Generate type helpers
+  //===--------------------------------------------------------------------===//
----------------
What are the possible parameter types for the type helper function gen? 


================
Comment at: flang/lib/Lower/ConvertType.cpp:301
+  mlir::Type gen(const Fortran::evaluate::BOZLiteralConstant &) {
+    return genTypelessPtr();
+  }
----------------
Why is it a TypelessPtr for BOZLiteralConstant?


================
Comment at: flang/lib/Lower/ConvertType.cpp:378
+        ty = rec;
+      } else {
+        emitError("symbol's type must have a type spec");
----------------
Are the polymorphic or class stars missing?


================
Comment at: flang/lib/Lower/ConvertType.cpp:388
+      if (symbol.GetType()->category() ==
+          Fortran::semantics::DeclTypeSpec::Character) {
+        auto charLen = fir::SequenceType::getUnknownExtent();
----------------
Isn't similar processing required for non-character sequences?
If yes, can a TODO be added?


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

https://reviews.llvm.org/D81034