[PATCH] D114534: [flang][runtime] Add ragged array runtime functions

[Peter Klausler via Phabricator via llvm-commits]

klausler added inline comments.


================
Comment at: flang/include/flang/Runtime/ragged.h:27
+struct RaggedArrayHeader {
+  std::uint64_t flags{0u};
+  void *bufferPointer{nullptr};
----------------
Could you convert the "flags" data member into a `bool` and a `std::uint8_t`?  It's just holding a flag and an integer in the range 1-15, if I understand this rightly.


================
Comment at: flang/runtime/ragged.cpp:14
+
+inline bool isIndirection(const RaggedArrayHeader *const header) {
+  return header->flags & 1;
----------------
These would both be better encoded as member functions, if that's possible.


================
Comment at: flang/runtime/ragged.cpp:25
+  if (header && rank) {
+    std::int64_t size = 1;
+    for (std::int64_t counter{0}; counter < rank; ++counter) {
----------------
Please use braced initialization in the runtime.


================
Comment at: flang/runtime/ragged.cpp:35
+    if (isHeader) {
+      header->bufferPointer = new RaggedArrayHeader[size];
+    } else {
----------------
When bufferPointer is allocated with `operator new []`, it should be deleted with `operator delete []`, not `std::free()` as is done below.  But maybe you should avoid the use of C++ runtime features to reduce the risk of making the Fortran runtime library dependent on a C++ runtime.  Can you rewrite as a call to `std::malloc()`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114534