shafik added inline comments.
================ Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:4667 + + SmallVector<uint64_t, 13> Expr; + AppendAddressSpaceXDeref(AddressSpace, Expr); ---------------- aprantl wrote: > 13 seems to be unnecessarily high. Shouldn't 1 be enough for the expected > single DW_OP_deref? Good catch, I used the `VarDecl` version of `EmitDeclare` as an example and I copied it from there. I will also be pushing the `DW_OP_plus_uconst` and the value later on as well. So maybe 3 is a the right value. ================ Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:4713 + .toCharUnitsFromBits(value * typeSize) + .getQuantity()); + } ---------------- aprantl wrote: > Can C++ arrays ever have a non-zero stride? (perhaps due to element > alignment?) Elements must be contiguous and start at `0` see [dcl.array/p6](http://eel.is/c++draft/dcl.array#6): >An object of type “array of N U” consists of a contiguously allocated >non-empty set of N subobjects of type U, known as the elements of the array, >and numbered 0 to N-1. The size of the type should include any padding. ================ Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:4716 + } + } + ---------------- aprantl wrote: > Should there be an else { assert("unhandled binding expressions"); } here or > are there other expressions that just don't need special handling? We should only have three cases: - struct - array - tuple-like We exit early for the tuple-like case at the top b/c we handle that elsewhere. So maybe asserting we never end up with a not handled case may make sense. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119178/new/ https://reviews.llvm.org/D119178 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
