[SILProfiler] Do not visit closure bodies twice

lib/SIL/SILProfiler.cpp

@@ -247,6 +247,11 @@ struct MapRegionCounters : public ASTWalker {
     if (skipExpr(E))
       return {true, E};
 
+    // Profiling for closures should be handled separately. Do not visit
+    // closure expressions twice.
+    if (isa<AbstractClosureExpr>(E) && !Parent.isNull())
+      return {false, E};
+
     // If AST visitation begins with an expression, the counter map must be
     // empty. Set up a counter for the root.
     if (Parent.isNull()) {
@@ -430,6 +435,20 @@ class SourceMappingRegion {
     assert(EndLoc && "Region has no end location");
     return *EndLoc;
   }
+
+  void print(llvm::raw_ostream &OS, const SourceManager &SM) const {
+    OS << "[";
+    if (hasStartLoc())
+      getStartLoc().print(OS, SM);
+    else
+      OS << "?";
+    OS << ", ";
+    if (hasEndLoc())
+      getEndLoc().print(OS, SM);
+    else
+      OS << "?";
+    OS << "]";
+  }
 };
 
 /// An ASTWalker that maps ASTNodes to profiling counters.
@@ -591,6 +610,11 @@ struct PGOMapping : public ASTWalker {
     if (skipExpr(E))
       return {true, E};
 
+    // Profiling for closures should be handled separately. Do not visit
+    // closure expressions twice.
+    if (isa<AbstractClosureExpr>(E) && !Parent.isNull())
+      return {false, E};
+
     unsigned parent = getParentCounter();
 
     if (Parent.isNull()) {
@@ -754,6 +778,11 @@ struct CoverageMapping : public ASTWalker {
   void pushRegion(ASTNode Node) {
     RegionStack.emplace_back(Node, getCounter(Node), Node.getStartLoc(),
                              getEndLoc(Node));
+    LLVM_DEBUG({
+      llvm::dbgs() << "Pushed region: ";
+      RegionStack.back().print(llvm::dbgs(), SM);
+      llvm::dbgs() << "\n";
+    });
   }
 
   /// Replace the current region's count by pushing an incomplete region.
@@ -780,6 +809,8 @@ struct CoverageMapping : public ASTWalker {
     auto ParentIt = I;
     SourceLoc EndLoc = ParentIt->getEndLoc();
 
+    unsigned FirstPoppedIndex = SourceRegions.size();
+    (void)FirstPoppedIndex;
     SourceRegions.push_back(std::move(*I++));
     for (; I != E; ++I) {
       if (!I->hasStartLoc())
@@ -789,6 +820,14 @@ struct CoverageMapping : public ASTWalker {
       SourceRegions.push_back(std::move(*I));
     }
 
+    LLVM_DEBUG({
+      for (unsigned Idx = FirstPoppedIndex; Idx < SourceRegions.size(); ++Idx) {
+        llvm::dbgs() << "Popped region: ";
+        SourceRegions[Idx].print(llvm::dbgs(), SM);
+        llvm::dbgs() << "\n";
+      }
+    });
+
     RegionStack.erase(ParentIt, E);
   }
 
@@ -1017,6 +1056,11 @@ struct CoverageMapping : public ASTWalker {
     if (skipExpr(E))
       return {true, E};
 
+    // Profiling for closures should be handled separately. Do not visit
+    // closure expressions twice.
+    if (isa<AbstractClosureExpr>(E) && !Parent.isNull())
+      return {false, E};
+
     if (!RegionStack.empty())
       extendRegion(E);
 

test/Profiler/coverage_empty_region_stack1.swift

@@ -0,0 +1,20 @@
+// RUN: %target-swift-frontend -Xllvm -sil-full-demangle -profile-generate -profile-coverage-mapping -emit-sil -module-name coverage_empty %s | %FileCheck %s
+
+// Skip the sil prologue, which reproduces the swift source of this file
+// and confounds FileCheck.
+// CHECK-LABEL: sil @main
+
+func singleDefaultArgument(i: Int = {
+  // CHECK: sil_coverage_map {{.*}}closure #1 () -> Swift.Int in default argument 0
+  // CHECK-NEXT:   [[@LINE-2]]:37 -> [[@LINE+6]]:2 : 0
+  // CHECK-NEXT: }
+  struct SingleDefaultArgumentStruct {
+    let sdasi: Int
+  }
+  return 2
+}()) {
+  // CHECK: sil_coverage_map {{.*}}singleDefaultArgument(i: Swift.Int) -> ()
+  // CHECK-NEXT:   [[@LINE-2]]:6 -> [[@LINE+3]]:2 : 0
+  // CHECK-NEXT: }
+  print(i)
+}