This is an archive of the discontinued LLVM Phabricator instance.

Add support for generating debug-info for structured bindings of structs and arrays
ClosedPublic

Authored by shafik on Feb 7 2022, 12:25 PM.

Details

Summary

Currently we are not emitting debug-info for all cases of structured bindings a C++17 feature which allows us to bind names to subobjects in an initializer.

A structured binding is represented by a DecompositionDecl AST node and the binding are represented by a BindingDecl. It looks the original implementation
only covered the tuple like case which be represented by a DeclRefExpr which contains a VarDecl.

If the binding is to a subobject of the struct the binding will contain a MemberExpr and in the case of arrays it will contain an ArraySubscriptExpr.
This PR adds support emitting debug-info for the MemberExpr and ArraySubscriptExpr cases as well as llvm and lldb tests for these cases as well as the tuple case.

Diff Detail

Event Timeline

shafik requested review of this revision.Feb 7 2022, 12:25 PM
shafik created this revision.

I think this looks pretty good! I have a few questions inside.

clang/lib/CodeGen/CGDebugInfo.cpp
4663

13 seems to be unnecessarily high. Shouldn't 1 be enough for the expected single DW_OP_deref?

4696

this looks good!

4709

Can C++ arrays ever have a non-zero stride? (perhaps due to element alignment?)

4712

Should there be an else { assert("unhandled binding expressions"); } here or are there other expressions that just don't need special handling?

clang/test/CodeGenCXX/debug-info-structured-binding.cpp
4

We should check what F is, too, right?

lldb/test/API/lang/cpp/structured-binding/main.cpp
70

Nice!

aprantl added inline comments.Feb 8 2022, 4:36 PM
clang/lib/CodeGen/CGDebugInfo.cpp
4643

do you need a

if (DebugKind > codegenoptions::LimitedDebugInfo)
 return

here?

shafik added inline comments.Feb 8 2022, 4:44 PM
clang/lib/CodeGen/CGDebugInfo.cpp
4663

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.

4709

Elements must be contiguous and start at 0 see dcl.array/p6:

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.

4712

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.

shafik updated this revision to Diff 409056.Feb 15 2022, 2:19 PM
shafik marked 5 inline comments as done.

Addressed comments on SmallVector size and fixed test.

clang/lib/CodeGen/CGDebugInfo.cpp
4643

That kind of check is only used in limited places, why would it apply here as let's say opposed to the EmitDeclare for VarDecl case?

clang/test/CodeGenCXX/debug-info-structured-binding.cpp
4

Actually I should have used a different match, that was a mistake.

aprantl accepted this revision.Feb 16 2022, 12:42 PM
aprantl added inline comments.
clang/lib/CodeGen/CGDebugInfo.cpp
4643

I think the answer is that Ty would be null then.

This revision is now accepted and ready to land.Feb 16 2022, 12:42 PM
This revision was landed with ongoing or failed builds.Feb 17 2022, 11:14 AM
This revision was automatically updated to reflect the committed changes.
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptFeb 17 2022, 11:14 AM