Skip to content

Commit ab63c6d

Browse files
committed
[DebugInfo] When referencing structured bindings use the reference's location, not the binding's declaration's location
For structured bindings that use custom `get` specializations, the resulting LLVM IR ascribes the load of the result of `get` to the binding's declaration, rather than the place where the binding is referenced - this caused awkward sequencing in the debug info where, when stepping through the code you'd step back to the binding declaration every time there was a reference to the binding. To fix that - when we cross into IRGening a binding - suppress the debug info location of that subexpression. I don't represent this as a great bit of API design - certainly open to ideas, but putting it out here as a place to start. It's /possible/ this is an incomplete fix, even - if the binding decl had other subexpressions, those would still get their location applied & it'd likely be wrong. So maybe that's a direction to go with to productionize this - add a new location scoped device that suppresses any overriding - this might be more robust. How do people feel about that?
1 parent 8071d27 commit ab63c6d

File tree

3 files changed

+20
-9
lines changed

3 files changed

+20
-9
lines changed

clang/lib/CodeGen/CGExpr.cpp

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1565,12 +1565,13 @@ LValue CodeGenFunction::EmitCheckedLValue(const Expr *E, TypeCheckKind TCK) {
15651565
/// length type, this is not possible.
15661566
///
15671567
LValue CodeGenFunction::EmitLValue(const Expr *E,
1568-
KnownNonNull_t IsKnownNonNull) {
1568+
KnownNonNull_t IsKnownNonNull, bool SuppressLocation) {
15691569
// Running with sufficient stack space to avoid deeply nested expressions
15701570
// cause a stack overflow.
15711571
LValue LV;
1572-
CGM.runWithSufficientStackSpace(
1573-
E->getExprLoc(), [&] { LV = EmitLValueHelper(E, IsKnownNonNull); });
1572+
CGM.runWithSufficientStackSpace(E->getExprLoc(), [&] {
1573+
LV = EmitLValueHelper(E, IsKnownNonNull, SuppressLocation);
1574+
});
15741575

15751576
if (IsKnownNonNull && !LV.isKnownNonNull())
15761577
LV.setKnownNonNull();
@@ -1586,8 +1587,11 @@ static QualType getConstantExprReferredType(const FullExpr *E,
15861587
}
15871588

15881589
LValue CodeGenFunction::EmitLValueHelper(const Expr *E,
1589-
KnownNonNull_t IsKnownNonNull) {
1590-
ApplyDebugLocation DL(*this, E);
1590+
KnownNonNull_t IsKnownNonNull,
1591+
bool SuppressLocation) {
1592+
std::optional<ApplyDebugLocation> DL;
1593+
if (!SuppressLocation)
1594+
DL.emplace(*this, E);
15911595
switch (E->getStmtClass()) {
15921596
default: return EmitUnsupportedLValue(E, "l-value expression");
15931597

@@ -3325,7 +3329,7 @@ LValue CodeGenFunction::EmitDeclRefLValue(const DeclRefExpr *E) {
33253329
auto *FD = LambdaCaptureFields.lookup(BD);
33263330
return EmitCapturedFieldLValue(*this, FD, CXXABIThisValue);
33273331
}
3328-
return EmitLValue(BD->getBinding());
3332+
return EmitLValue(BD->getBinding(), NotKnownNonNull, true);
33293333
}
33303334

33313335
// We can form DeclRefExprs naming GUID declarations when reconstituting

clang/lib/CodeGen/CodeGenFunction.h

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4225,10 +4225,12 @@ class CodeGenFunction : public CodeGenTypeCache {
42254225
/// variable length type, this is not possible.
42264226
///
42274227
LValue EmitLValue(const Expr *E,
4228-
KnownNonNull_t IsKnownNonNull = NotKnownNonNull);
4228+
KnownNonNull_t IsKnownNonNull = NotKnownNonNull,
4229+
bool SuppressLocation = false);
42294230

42304231
private:
4231-
LValue EmitLValueHelper(const Expr *E, KnownNonNull_t IsKnownNonNull);
4232+
LValue EmitLValueHelper(const Expr *E, KnownNonNull_t IsKnownNonNull,
4233+
bool SuppressLocation);
42324234

42334235
public:
42344236
/// Same as EmitLValue but additionally we generate checking code to

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
// CHECK: #dbg_declare(ptr %{{[0-9]+}}, ![[VAR_4:[0-9]+]], !DIExpression(DW_OP_deref, DW_OP_plus_uconst, 4),
88
// CHECK: #dbg_declare(ptr %z1, ![[VAR_5:[0-9]+]], !DIExpression()
99
// CHECK: #dbg_declare(ptr %z2, ![[VAR_6:[0-9]+]], !DIExpression()
10+
// CHECK: load ptr, ptr %z1, {{.*}}!dbg ![[Z1_DEBUG_LOC:[0-9]+]]
1011
// CHECK: ![[VAR_0]] = !DILocalVariable(name: "a"
1112
// CHECK: ![[VAR_1]] = !DILocalVariable(name: "x1", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: {{[0-9]+}}, type: !{{[0-9]+}})
1213
// CHECK: ![[VAR_2]] = !DILocalVariable(name: "y1", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: {{[0-9]+}}, type: !{{[0-9]+}})
@@ -46,5 +47,9 @@ int f() {
4647
auto [x1, y1] = a;
4748
auto &[x2, y2] = a;
4849
auto [z1, z2] = B{1, 2};
49-
return x1 + y1 + x2 + y2 + z1 + z2;
50+
return x1 + y1 + x2 + y2 +
51+
// CHECK: ![[Z1_DEBUG_LOC]] = !DILocation(line: [[@LINE+1]]
52+
z1 //
53+
+ //
54+
z2; //
5055
}

0 commit comments

Comments
 (0)