[PATCH] D48024: [DebugInfo] Check size of variable in ConvertDebugDeclareToDebugValue

[Adrian Prantl via Phabricator via llvm-commits]

aprantl added inline comments.


================
Comment at: lib/Transforms/Utils/Local.cpp:1231
 
+/// Get the number of bits that are described by a dbg.declare.
+static uint64_t DescribedVariablesSizeInBits(DbgInfoIntrinsic *DII) {
----------------
`... by a llvm.dbg intrinsic.`


================
Comment at: lib/Transforms/Utils/Local.cpp:1232
+/// Get the number of bits that are described by a dbg.declare.
+static uint64_t DescribedVariablesSizeInBits(DbgInfoIntrinsic *DII) {
+  if (auto Fragment = DII->getExpression()->getFragmentInfo())
----------------
I think that `getSizeOfFragment()` would be more a more accurate name?


================
Comment at: lib/Transforms/Utils/Local.cpp:1237
+    return *VarSize;
+  llvm_unreachable("Unkonwn size of the variable described by DII");
+}
----------------
Unknown

Is this really unreachable, i.e.: would the Verifier reject a variable with a size of zero? In C (but not C++) I believe the size of `struct Empty {};` is zero. In Swift you can also have zero-sized types.


================
Comment at: lib/Transforms/Utils/Local.cpp:1242
+/// \p DII isn't greater than the size of \p Ty.
+static bool IsVariableCoveredByType(DbgInfoIntrinsic *DII, Type *Ty) {
+  const DataLayout &DL = DII->getModule()->getDataLayout();
----------------
How about: `fragmentCoversEntireVariable()`


================
Comment at: lib/Transforms/Utils/Local.cpp:1272
+                                      SI);
+    return;
+  }
----------------
This part seems reasonable (though sort of unsatisfying :-).


Repository:
  rL LLVM

https://reviews.llvm.org/D48024