[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 ↗

(On Diff #132464)

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 ↗

(On Diff #132464)

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!

SemaStmt.cpp

 
 /// 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);

       return StmtError();
   }
 
-  // Build  auto && __range = range-init
+  // Build  auto && __range = range-init.
+  // Assume the variables are nested in the inner scope (loop body).
+  const auto DepthStr = std::to_string(S->getDepth() >> 1);
   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();
 
     // Build auto __begin = begin-expr, __end = end-expr.
+    // Assume the variables are nested in the inner scope (loop body).
+    const auto DepthStr = std::to_string(S->getDepth() >> 1);
     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;