[Debug] Annotate compiler generated range-for loop variables.

[mattd]

Summary

This change aims to simplify debugging by annotating the range-for loop artificial variables (range, begin, end) with the scope depth.

[mattd]

Updating the diff, missed a few deltas that should have been in the original patch.

[dblaikie]

Seems plausible - maybe walking the scopes to count the range-for loops would produce a better number, but would be slower. :/ Dunno.

lib/Sema/SemaStmt.cpp

2346

Why shifted right/divided by two? (I'd probably write this as "/ 2" rather thane ">> 1" if this is what's)

I guess this is because a range-for introduces two (actually 3, I think) scopes?

But what about if there are other random scopes that could be present? Does that adversely affect anything here?

[mattd]

lib/Sema/SemaStmt.cpp

2346

Thanks for the comment. Honestly, I discovered the pattern after crafting up a tightly nested loop example, and realized that my Depths were 2,4,6... for each range loop respectively. I think it's two scopes and the range variables (begin, end, __range) are nestled inside each inner scope. Thus each range loop seems to be created of 2 scopes.

If there are other scopes between two range-loops, such as the 'if' in the example below, then the outer-most "for" is range1/begin1/end1, followed by the middle loop: range3/begin3/end3, and the innermost: range4/begin4/end4.

for (...) // range1
  if (...) 
    for(...) // range 3
      for (...) //range4

[mattd]

• Added a division by 2 instead of the shift, it reads clearer.
• Updated the associated comment, reflecting that we divide by two because the variables we are annotating are within the inner scope of the ranged-based for loop.

[dblaikie]

Seems good, thanks :)

test/CodeGenCXX/debug-for-range-scope-hints.cpp

1 ↗

(On Diff #134263)

maybe this test should be called debug-info-range-for-var-names? Just a thought. (debug-info seems to be the usual prefix here, and range-for is a common shortening of "range-based-for" I think (maybe I'm wrong & other tests don't use that naming?)? & 'hints' seems a bit vague as to what the test is about - when it's specifically about naming the variables)

28–36 ↗

(On Diff #134263)

I'd drop the ", {{.*}}, flags: DIFlagArtificial)" from these lines - since the name's the only bit that's really interesting to this test.

[mattd]

Thanks @dblaikie! I renamed the test, and cleaned up per your suggestion. I originally regex'd the debug-info lines so that the test would verify that the names were artificial; however, being that we already match them as metadata earlier, it's not really that necessary; we are only testing name strings anyways. Thanks again for the suggestion.

[dblaikie]

In D42813#1007865, @mattd wrote:

Thanks @dblaikie! I renamed the test, and cleaned up per your suggestion. I originally regex'd the debug-info lines so that the test would verify that the names were artificial; however, being that we already match them as metadata earlier, it's not really that necessary; we are only testing name strings anyways. Thanks again for the suggestion.

Great - can you commit this yourself or would you like me to do it for you?

[mattd]

Great - can you commit this yourself or would you like me to do it for you?

I've got it. Thanks!

include/clang/Sema/Scope.h

public:
   }
 
   Scope *getBlockParent() { return BlockParent; }
   const Scope *getBlockParent() const { return BlockParent; }
 
   Scope *getTemplateParamParent() { return TemplateParamParent; }
   const Scope *getTemplateParamParent() const { return TemplateParamParent; }
 
+  /// Returns the depth of this scope. The translation-unit has scope depth 0.
+  unsigned getDepth() const { return Depth; }
+
   /// Returns the number of function prototype scopes in this scope
   /// chain.
   unsigned getFunctionPrototypeDepth() const {
     return PrototypeDepth;
   }
 
   /// Return the number of parameters declared in this function
   /// prototype, increasing it by one for the next call.

lib/Sema/SemaStmt.cpp

void NoteForRangeBeginEndFunction(Sema &SemaRef, Expr *E,
   }
 
   SemaRef.Diag(Loc, diag::note_for_range_begin_end)
     << BEF << IsTemplate << Description << E->getType();
 }
 
 /// Build a variable declaration for a for-range statement.
 VarDecl *BuildForRangeVarDecl(Sema &SemaRef, SourceLocation Loc,
-                              QualType Type, const char *Name) {
+                              QualType Type, StringRef Name) {
   DeclContext *DC = SemaRef.CurContext;
   IdentifierInfo *II = &SemaRef.PP.getIdentifierTable().get(Name);
   TypeSourceInfo *TInfo = SemaRef.Context.getTrivialTypeSourceInfo(Type, Loc);
   VarDecl *Decl = VarDecl::Create(SemaRef.Context, DC, Loc, Loc, II, Type,
                                   TInfo, SC_None);
   Decl->setImplicit();
   return Decl;
 }
StmtResult Sema::ActOnCXXForRangeStmt(Scope *S, SourceLocation ForLoc,
   // Build the coroutine state immediately and not later during template
   // instantiation
   if (!CoawaitLoc.isInvalid()) {
     if (!ActOnCoroutineBodyStart(S, CoawaitLoc, "co_await"))
       return StmtError();
   }
 
   // Build  auto && __range = range-init
+  // Divide by 2, since the variables are in the inner scope (loop body).
+  const auto DepthStr = std::to_string(S->getDepth() / 2);
   SourceLocation RangeLoc = Range->getLocStart();
   VarDecl *RangeVar = BuildForRangeVarDecl(*this, RangeLoc,
                                            Context.getAutoRRefDeductType(),
-                                           "__range");
+                                           std::string("__range") + DepthStr);
   if (FinishForRangeVarDecl(*this, RangeVar, Range, RangeLoc,
                             diag::err_for_range_deduction_failure)) {
     LoopVar->setInvalidDecl();
     return StmtError();
   }
 
   // Claim the type doesn't contain auto: we've already done the checking.
   DeclGroupPtrTy RangeGroup =
if (!Range)
       return StmtError();
     QualType RangeType = Range->getType();
 
     if (RequireCompleteType(RangeLoc, RangeType,
                             diag::err_for_range_incomplete_type))
       return StmtError();
 
     // Build auto __begin = begin-expr, __end = end-expr.
+    // Divide by 2, since the variables are in the inner scope (loop body).
+    const auto DepthStr = std::to_string(S->getDepth() / 2);
     VarDecl *BeginVar = BuildForRangeVarDecl(*this, ColonLoc, AutoType,
-                                             "__begin");
+                                             std::string("__begin") + DepthStr);
     VarDecl *EndVar = BuildForRangeVarDecl(*this, ColonLoc, AutoType,
-                                           "__end");
+                                           std::string("__end") + DepthStr);
 
     // Build begin-expr and end-expr and attach to __begin and __end variables.
     ExprResult BeginExpr, EndExpr;
     if (const ArrayType *UnqAT = RangeType->getAsArrayTypeUnsafe()) {
       // - if _RangeT is an array type, begin-expr and end-expr are __range and
       //   __range + __bound, respectively, where __bound is the array bound. If
       //   _RangeT is an array of unknown size or an array of incomplete type,
       //   the program is ill-formed;

test/CodeGenCXX/debug-info-range-for-var-names.cpp

+// RUN: %clang_cc1 -emit-llvm -debug-info-kind=limited %s -o - | FileCheck %s
+
+struct vec {
+  using itr = int*;
+  itr begin() { return nullptr; }
+  itr end() { return nullptr; }
+};
+
+void test() {
+  vec as, bs, cs;
+
+  for (auto a : as)
+    for (auto b : bs)
+      for (auto c : cs) {
+      }
+}
+
+// CHECK: define void @_Z4testv()
+// CHECK: call void @llvm.dbg.declare(metadata %struct.vec** {{.*}}, metadata ![[RANGE1:[0-9]+]]
+// CHECK: call void @llvm.dbg.declare(metadata i32** {{.*}}, metadata ![[BEGIN1:[0-9]+]]
+// CHECK: call void @llvm.dbg.declare(metadata i32** {{.*}}, metadata ![[END1:[0-9]+]]
+// CHECK: call void @llvm.dbg.declare(metadata %struct.vec** {{.*}}, metadata ![[RANGE2:[0-9]+]]
+// CHECK: call void @llvm.dbg.declare(metadata i32** {{.*}}, metadata ![[BEGIN2:[0-9]+]]
+// CHECK: call void @llvm.dbg.declare(metadata i32** {{.*}}, metadata ![[END2:[0-9]+]]
+// CHECK: call void @llvm.dbg.declare(metadata %struct.vec** {{.*}}, metadata ![[RANGE3:[0-9]+]]
+// CHECK: call void @llvm.dbg.declare(metadata i32** {{.*}}, metadata ![[BEGIN3:[0-9]+]]
+// CHECK: call void @llvm.dbg.declare(metadata i32** {{.*}}, metadata ![[END3:[0-9]+]]
+// CHECK: ![[RANGE1]] = !DILocalVariable(name: "__range1",
+// CHECK: ![[BEGIN1]] = !DILocalVariable(name: "__begin1",
+// CHECK: ![[END1]] = !DILocalVariable(name: "__end1",
+// CHECK: ![[RANGE2]] = !DILocalVariable(name: "__range2",
+// CHECK: ![[BEGIN2]] = !DILocalVariable(name: "__begin2",
+// CHECK: ![[END2]] = !DILocalVariable(name: "__end2",
+// CHECK: ![[RANGE3]] = !DILocalVariable(name: "__range3",
+// CHECK: ![[BEGIN3]] = !DILocalVariable(name: "__begin3",
+// CHECK: ![[END3]] = !DILocalVariable(name: "__end3",

test/CodeGenCXX/debug-info-scope.cpp

for (int i = 0; i != 10; ++i) {
     // CHECK: = !DILocalVariable(name: "b"
     // CHECK-SAME:               scope: [[FOR_COMPOUND:![0-9]*]]
     // CHECK-SAME:               line: [[@LINE+2]]
     // CHECK: [[FOR_COMPOUND]] = distinct !DILexicalBlock(scope: [[FOR_LOOP_INCLUDING_COND]],{{.*}} line: [[@LINE-8]])
     bool b = i % 2;
   }
 
   int x[] = {1, 2};
-  // CHECK: = !DILocalVariable(name: "__range"
+  // CHECK: = !DILocalVariable(name: "__range1"
   // CHECK-SAME:               scope: [[RANGE_FOR:![0-9]*]]
   // CHECK-NOT:                line:
   // CHECK-SAME:               ){{$}}
   // CHECK: [[RANGE_FOR]] = distinct !DILexicalBlock({{.*}}, line: [[@LINE+1]])
   for (int i : x) {
     // CHECK: = !DILocalVariable(name: "i"
     // CHECK-SAME:               scope: [[RANGE_FOR_BODY:![0-9]*]]
     // CHECK-SAME:               line: [[@LINE-3]]
     // CHECK: [[RANGE_FOR_BODY]] = distinct !DILexicalBlock(scope: [[RANGE_FOR]],{{.*}} line: [[@LINE-4]])
   }
 }

test/CodeGenCXX/vla.cpp

void test0(void *array, int n) {
 
   // CHECK-NEXT: ret void
 }
 
 
 void test2(int b) {
   // CHECK-LABEL: define void {{.*}}test2{{.*}}(i32 %b)
   int varr[b];
-  // AMD: %__end = alloca i32*, align 8, addrspace(5)
-  // AMD: [[END:%.*]] = addrspacecast i32* addrspace(5)* %__end to i32**
+  // AMD: %__end1 = alloca i32*, align 8, addrspace(5)
+  // AMD: [[END:%.*]] = addrspacecast i32* addrspace(5)* %__end1 to i32**
   // get the address of %b by checking the first store that stores it 
   //CHECK: store i32 %b, i32* [[PTR_B:%.*]]
 
   // get the size of the VLA by getting the first load of the PTR_B
   //CHECK: [[VLA_NUM_ELEMENTS_PREZEXT:%.*]] = load i32, i32* [[PTR_B]]
   //CHECK-NEXT: [[VLA_NUM_ELEMENTS_PRE:%.*]] = zext i32 [[VLA_NUM_ELEMENTS_PREZEXT]]
   
   b = 15;
   //CHECK: store i32 15, i32* [[PTR_B]]
   
   // Now get the sizeof, and then divide by the element size
   
   
   //CHECK: [[VLA_SIZEOF:%.*]] = mul nuw i64 4, [[VLA_NUM_ELEMENTS_PRE]]
   //CHECK-NEXT: [[VLA_NUM_ELEMENTS_POST:%.*]] = udiv i64 [[VLA_SIZEOF]], 4
   //CHECK-NEXT: [[VLA_END_PTR:%.*]] = getelementptr inbounds i32, i32* {{%.*}}, i64 [[VLA_NUM_ELEMENTS_POST]]
-  //X64-NEXT: store i32* [[VLA_END_PTR]], i32** %__end
+  //X64-NEXT: store i32* [[VLA_END_PTR]], i32** %__end1
   //AMD-NEXT: store i32* [[VLA_END_PTR]], i32** [[END]]
   for (int d : varr) 0;
 }
 
 void test3(int b, int c) {
   // CHECK-LABEL: define void {{.*}}test3{{.*}}(i32 %b, i32 %c)
   int varr[b][c];
-  // AMD: %__end = alloca i32*, align 8, addrspace(5)
-  // AMD: [[END:%.*]] = addrspacecast i32* addrspace(5)* %__end to i32**
+  // AMD: %__end1 = alloca i32*, align 8, addrspace(5)
+  // AMD: [[END:%.*]] = addrspacecast i32* addrspace(5)* %__end1 to i32**
   // get the address of %b by checking the first store that stores it 
   //CHECK: store i32 %b, i32* [[PTR_B:%.*]]
   //CHECK-NEXT: store i32 %c, i32* [[PTR_C:%.*]]
   
   // get the size of the VLA by getting the first load of the PTR_B
   //CHECK: [[VLA_DIM1_PREZEXT:%.*]] = load i32, i32* [[PTR_B]]
   //CHECK-NEXT: [[VLA_DIM1_PRE:%.*]] = zext i32 [[VLA_DIM1_PREZEXT]]
   //CHECK: [[VLA_DIM2_PREZEXT:%.*]] = load i32, i32* [[PTR_C]]