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

[Bjorn Pettersson via Phabricator via llvm-commits]

bjope added inline comments.


================
Comment at: lib/Transforms/Utils/Local.cpp:1237
+    return *VarSize;
+  llvm_unreachable("Unkonwn size of the variable described by DII");
+}
----------------
aprantl wrote:
> 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.
We only end up in the unreachable if getSizeInBits failed to determine the size (it returns an Optional<uint64_t>). So *VarSize could still be zero.

But maybe we need to handle the situation more gracefully. I should probably let this method return an Optional<uint64_t> as well, and then deal with it somehow in IsVariableCoveredByType.


================
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();
----------------
aprantl wrote:
> How about: `fragmentCoversEntireVariable()`
In my world the variable and the fragment is both related to the same thing.
But I can change it to: valueCoversEntireFragment(Type *Ty, DbgInfoIntrinsic *DII)


Repository:
  rL LLVM

https://reviews.llvm.org/D48024