Skip to content

Commit 310a9f3

Browse files
authored
[clang][DebugInfo] Don't mark structured bindings as artificial (#100355)
This patch is motivated by the debug-info issue in #48909. Clang is currently emitting the `DW_AT_artificial` attribute on debug-info entries for structured bindings whereas GCC does not. GCC's interpretation of the DWARF spec is more user-friendly in this regard, so we would like to do the same in Clang. [`CGDebugInfo` uses `isImplicit` to decide which variables to mark artificial](https://github.com/llvm/llvm-project/blob/0c4023ae3b64c54ff51947e9776aee0e963c5635/clang/lib/CodeGen/CGDebugInfo.cpp#L4783-L4784) (e.g., `ImplicitParamDecls`, compiler-generated variables, etc.). But for the purposes of debug-info, when we say "artificial", what we really mean in many cases is: "not explicitly spelled out in source". `VarDecl`s that back tuple-like bindings are [technically compiler-generated](#48909 (comment)), but that is a confusing notion for debug-info, since these bindings *are* spelled out in source. The [documentation for `isImplicit`](https://github.com/llvm/llvm-project/blob/68a0d0c76223736351fd7c452bca3ba9d80ca342/clang/include/clang/AST/DeclBase.h#L596-L600) does to some extent imply that implicit variables aren't written in source. This patch adds another condition to deciding whether a `VarDecl` should be marked artificial. Specifically, don't treat structured bindings as artificial. **Main alternatives considered** 1. Don't use `isImplicit` in `CGDebugInfo` when determining whether to add `DW_AT_artificial`. Instead use some other property of the AST that would tell us whether a node was explicitly spelled out in source or not * Considered using `SourceRange` or `SourceLocation` to tell us this, but didn't find a good way to, e.g., correctly identify that the implicit `this` parameter wasn't spelled out in source (as opposed to an unnamed parameter in a function declaration) 2. We could've also added a bit to `VarDeclBitFields` that indicates that a `VarDecl` is a holding var, but the use-case didn't feel like good enough justification for this 3. Don't set the `VarDecl` introduced as part of a tuple-like decomposition as implicit. * This may affect AST matching/traversal and this specific use-case wasn't enough to justify such a change Fixes #48909
1 parent 3b175e1 commit 310a9f3

File tree

2 files changed

+66
-11
lines changed

2 files changed

+66
-11
lines changed

clang/lib/CodeGen/CGDebugInfo.cpp

Lines changed: 35 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
#include "TargetInfo.h"
2222
#include "clang/AST/ASTContext.h"
2323
#include "clang/AST/Attr.h"
24+
#include "clang/AST/DeclCXX.h"
2425
#include "clang/AST/DeclFriend.h"
2526
#include "clang/AST/DeclObjC.h"
2627
#include "clang/AST/DeclTemplate.h"
@@ -79,6 +80,35 @@ static uint32_t getDeclAlignIfRequired(const Decl *D, const ASTContext &Ctx) {
7980
return D->hasAttr<AlignedAttr>() ? D->getMaxAlignment() : 0;
8081
}
8182

83+
/// Returns true if \ref VD is a a holding variable (aka a
84+
/// VarDecl retrieved using \ref BindingDecl::getHoldingVar).
85+
static bool IsDecomposedVarDecl(VarDecl const *VD) {
86+
auto const *Init = VD->getInit();
87+
if (!Init)
88+
return false;
89+
90+
auto const *RefExpr =
91+
llvm::dyn_cast_or_null<DeclRefExpr>(Init->IgnoreUnlessSpelledInSource());
92+
if (!RefExpr)
93+
return false;
94+
95+
return llvm::dyn_cast_or_null<DecompositionDecl>(RefExpr->getDecl());
96+
}
97+
98+
/// Returns true if \ref VD is a compiler-generated variable
99+
/// and should be treated as artificial for the purposes
100+
/// of debug-info generation.
101+
static bool IsArtificial(VarDecl const *VD) {
102+
// Tuple-like bindings are marked as implicit despite
103+
// being spelled out in source. Don't treat them as artificial
104+
// variables.
105+
if (IsDecomposedVarDecl(VD))
106+
return false;
107+
108+
return VD->isImplicit() || (isa<Decl>(VD->getDeclContext()) &&
109+
cast<Decl>(VD->getDeclContext())->isImplicit());
110+
}
111+
82112
CGDebugInfo::CGDebugInfo(CodeGenModule &CGM)
83113
: CGM(CGM), DebugKind(CGM.getCodeGenOpts().getDebugInfo()),
84114
DebugTypeExtRefs(CGM.getCodeGenOpts().DebugTypeExtRefs),
@@ -4766,11 +4796,10 @@ llvm::DILocalVariable *CGDebugInfo::EmitDeclare(const VarDecl *VD,
47664796
if (VD->hasAttr<NoDebugAttr>())
47674797
return nullptr;
47684798

4769-
bool Unwritten =
4770-
VD->isImplicit() || (isa<Decl>(VD->getDeclContext()) &&
4771-
cast<Decl>(VD->getDeclContext())->isImplicit());
4799+
const bool VarIsArtificial = IsArtificial(VD);
4800+
47724801
llvm::DIFile *Unit = nullptr;
4773-
if (!Unwritten)
4802+
if (!VarIsArtificial)
47744803
Unit = getOrCreateFile(VD->getLocation());
47754804
llvm::DIType *Ty;
47764805
uint64_t XOffset = 0;
@@ -4787,13 +4816,13 @@ llvm::DILocalVariable *CGDebugInfo::EmitDeclare(const VarDecl *VD,
47874816
// Get location information.
47884817
unsigned Line = 0;
47894818
unsigned Column = 0;
4790-
if (!Unwritten) {
4819+
if (!VarIsArtificial) {
47914820
Line = getLineNumber(VD->getLocation());
47924821
Column = getColumnNumber(VD->getLocation());
47934822
}
47944823
SmallVector<uint64_t, 13> Expr;
47954824
llvm::DINode::DIFlags Flags = llvm::DINode::FlagZero;
4796-
if (VD->isImplicit())
4825+
if (VarIsArtificial)
47974826
Flags |= llvm::DINode::FlagArtificial;
47984827

47994828
auto Align = getDeclAlignIfRequired(VD, CGM.getContext());

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

Lines changed: 31 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,20 +5,46 @@
55
// CHECK: #dbg_declare(ptr %{{[0-9]+}}, ![[VAR_2:[0-9]+]], !DIExpression(DW_OP_plus_uconst, 4),
66
// CHECK: #dbg_declare(ptr %{{[0-9]+}}, ![[VAR_3:[0-9]+]], !DIExpression(DW_OP_deref),
77
// CHECK: #dbg_declare(ptr %{{[0-9]+}}, ![[VAR_4:[0-9]+]], !DIExpression(DW_OP_deref, DW_OP_plus_uconst, 4),
8+
// CHECK: #dbg_declare(ptr %z1, ![[VAR_5:[0-9]+]], !DIExpression()
9+
// CHECK: #dbg_declare(ptr %z2, ![[VAR_6:[0-9]+]], !DIExpression()
810
// CHECK: ![[VAR_0]] = !DILocalVariable(name: "a"
9-
// CHECK: ![[VAR_1]] = !DILocalVariable(name: "x1"
10-
// CHECK: ![[VAR_2]] = !DILocalVariable(name: "y1"
11-
// CHECK: ![[VAR_3]] = !DILocalVariable(name: "x2"
12-
// CHECK: ![[VAR_4]] = !DILocalVariable(name: "y2"
11+
// CHECK: ![[VAR_1]] = !DILocalVariable(name: "x1", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: {{[0-9]+}}, type: !{{[0-9]+}})
12+
// CHECK: ![[VAR_2]] = !DILocalVariable(name: "y1", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: {{[0-9]+}}, type: !{{[0-9]+}})
13+
// CHECK: ![[VAR_3]] = !DILocalVariable(name: "x2", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: {{[0-9]+}}, type: !{{[0-9]+}})
14+
// CHECK: ![[VAR_4]] = !DILocalVariable(name: "y2", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: {{[0-9]+}}, type: !{{[0-9]+}})
15+
// CHECK: ![[VAR_5]] = !DILocalVariable(name: "z1", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: {{[0-9]+}}, type: !{{[0-9]+}})
16+
// CHECK: ![[VAR_6]] = !DILocalVariable(name: "z2", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: {{[0-9]+}}, type: !{{[0-9]+}})
1317

1418
struct A {
1519
int x;
1620
int y;
1721
};
1822

23+
struct B {
24+
int w;
25+
int z;
26+
template<int> int get();
27+
template<> int get<0>() { return w; }
28+
template<> int get<1>() { return z; }
29+
};
30+
31+
// Note: the following declarations are necessary for decomposition of tuple-like
32+
// structured bindings
33+
namespace std {
34+
template<typename T> struct tuple_size {
35+
};
36+
template<>
37+
struct tuple_size<B> {
38+
static constexpr unsigned value = 2;
39+
};
40+
41+
template<unsigned, typename T> struct tuple_element { using type = int; };
42+
} // namespace std
43+
1944
int f() {
2045
A a{10, 20};
2146
auto [x1, y1] = a;
2247
auto &[x2, y2] = a;
23-
return x1 + y1 + x2 + y2;
48+
auto [z1, z2] = B{1, 2};
49+
return x1 + y1 + x2 + y2 + z1 + z2;
2450
}

0 commit comments

Comments
 (0)