[PATCH] D80197: [DebugInfo] Upgrade DISubrange to support Fortran dynamic arrays

[Adrian Prantl via Phabricator via llvm-commits]

aprantl added inline comments.


================
Comment at: clang/test/CodeGen/debug-info-matrix-types.c:14
+  // CHECK: [[COLS]] = !DISubrange(count: 3, lowerBound: 0)
+  // CHECK: [[ROWS]] = !DISubrange(count: 2, lowerBound: 0)
   // CHECK: [[EXPR_A]] = !DILocalVariable(name: "a", arg: 1, {{.+}} type: [[PTR_TY]])
----------------
Should we just make lowerBound implicit and only print it when it is non-null?


================
Comment at: llvm/lib/IR/AsmWriter.cpp:1866
+  // not be assumed as 0, as it depends on other factors like language, and
+  // it can also be calcualted as (upperBound - count + 1)
+  auto *LBound = N->getRawLowerBound();
----------------
`calculated`

I think this comment is wrong, since it doesn't account for stride.

What about: `A lowerBound of constant 0 should not be skipped, since it is different from an unspecified lower bound (= nullptr)`.

Similarly, should the existing use-cases in the test suite be nullptr values or constant zero?


================
Comment at: llvm/unittests/IR/MetadataTest.cpp:1266
+  EXPECT_NE(N, DISubrange::get(Context, nullptr, LV, UV, SEother));
+  EXPECT_NE(N, DISubrange::get(Context, nullptr, LV, UV, SIother));
+}
----------------
Should we also test that LV/UV of 0 is distinct from nullptr?


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

https://reviews.llvm.org/D80197