Fix debug description for cases with multiple items

include/swift/SIL/SILBuilder.h

@@ -899,6 +899,14 @@ class SILBuilder {
   DebugValueAddrInst *createDebugValueAddr(SILLocation Loc, SILValue src,
                                            SILDebugVariable Var);
 
+  /// Create a debug_value_addr if \p src is an address; a debug_value if not.
+  SILInstruction *emitDebugDescription(SILLocation Loc, SILValue src,
+                                       SILDebugVariable Var) {
+    if (src->getType().isAddress())
+      return createDebugValueAddr(Loc, src, Var);
+    return createDebugValue(Loc, src, Var);
+  }
+
 #define NEVER_LOADABLE_CHECKED_REF_STORAGE(Name, ...) \
   Load##Name##Inst *createLoad##Name(SILLocation Loc, \
                                      SILValue src, \

include/swift/SIL/SILDebugScope.h

@@ -56,6 +56,8 @@ class SILDebugScope : public SILAllocated<SILDebugScope> {
   /// Create a scope for an artificial function.
   SILDebugScope(SILLocation Loc);
 
+  SILLocation getLoc() const { return Loc; }
+
   /// Return the function this scope originated from before being inlined.
   SILFunction *getInlinedFunction() const;
 
@@ -64,12 +66,10 @@ class SILDebugScope : public SILAllocated<SILDebugScope> {
   /// into.
   SILFunction *getParentFunction() const;
 
-#ifndef NDEBUG
-  SWIFT_DEBUG_DUMPER(dump(SourceManager &SM,
-                          llvm::raw_ostream &OS = llvm::errs(),
-                          unsigned Indent = 0));
-  SWIFT_DEBUG_DUMPER(dump(SILModule &Mod));
-#endif
+  void print(SourceManager &SM, llvm::raw_ostream &OS = llvm::errs(),
+             unsigned Indent = 0) const;
+
+  void print(SILModule &Mod) const;
 };
 
 /// Determine whether an instruction may not have a SILDebugScope.

include/swift/SIL/SILLocation.h

@@ -490,6 +490,8 @@ class SILLocation {
            Loc.ASTNode.ForDebugger.getOpaqueValue() ==
                R.Loc.ASTNode.ForDebugger.getOpaqueValue();
   }
+
+  inline bool operator!=(const SILLocation &R) const { return !(*this == R); }
 };
 
 /// Allowed on any instruction.

lib/IRGen/IRGenSIL.cpp

@@ -696,6 +696,29 @@ class IRGenSILFunction :
     return !isa<llvm::Constant>(Storage);
   }
 
+#ifndef NDEBUG
+  /// Check if \p Val can be stored into \p Alloca, and emit some diagnostic
+  /// info if it can't.
+  bool canAllocaStoreValue(Address Alloca, llvm::Value *Val,
+                           SILDebugVariable VarInfo,
+                           const SILDebugScope *Scope) {
+    bool canStore =
+        cast<llvm::PointerType>(Alloca->getType())->getElementType() ==
+        Val->getType();
+    if (canStore)
+      return true;
+    llvm::errs() << "Invalid shadow copy:\n"
+                 << "  Value : " << *Val << "\n"
+                 << "  Alloca: " << *Alloca.getAddress() << "\n"
+                 << "---\n"
+                 << "Previous shadow copy into " << VarInfo.Name
+                 << " in the same scope!\n"
+                 << "Scope:\n";
+    Scope->print(getSILModule());
+    return false;
+  }
+#endif
+
   /// Unconditionally emit a stack shadow copy of an \c llvm::Value.
   llvm::Value *emitShadowCopy(llvm::Value *Storage, const SILDebugScope *Scope,
                               SILDebugVariable VarInfo, llvm::Optional<Alignment> _Align) {
@@ -706,7 +729,8 @@ class IRGenSILFunction :
     if (!Alloca.isValid())
       Alloca = createAlloca(Storage->getType(), Align, VarInfo.Name + ".debug");
     zeroInit(cast<llvm::AllocaInst>(Alloca.getAddress()));
-
+    assert(canAllocaStoreValue(Alloca, Storage, VarInfo, Scope) &&
+           "bad scope?");
     ArtificialLocation AutoRestore(Scope, IGM.DebugInfo.get(), Builder);
     Builder.CreateStore(Storage, Alloca.getAddress(), Align);
     return Alloca.getAddress();

lib/SIL/IR/SILPrinter.cpp

@@ -3381,7 +3381,7 @@ void SILCoverageMap::dump() const {
 #pragma warning(disable : 4996)
 #endif
 
-void SILDebugScope::dump(SourceManager &SM, llvm::raw_ostream &OS,
+void SILDebugScope::print(SourceManager &SM, llvm::raw_ostream &OS,
                          unsigned Indent) const {
   OS << "{\n";
   OS.indent(Indent);
@@ -3391,7 +3391,7 @@ void SILDebugScope::dump(SourceManager &SM, llvm::raw_ostream &OS,
   OS.indent(Indent + 2);
   OS << " parent: ";
   if (auto *P = Parent.dyn_cast<const SILDebugScope *>()) {
-    P->dump(SM, OS, Indent + 2);
+    P->print(SM, OS, Indent + 2);
     OS.indent(Indent + 2);
   }
   else if (auto *F = Parent.dyn_cast<SILFunction *>())
@@ -3403,15 +3403,15 @@ void SILDebugScope::dump(SourceManager &SM, llvm::raw_ostream &OS,
   OS.indent(Indent + 2);
   if (auto *CS = InlinedCallSite) {
     OS << "inlinedCallSite: ";
-    CS->dump(SM, OS, Indent + 2);
+    CS->print(SM, OS, Indent + 2);
     OS.indent(Indent + 2);
   }
   OS << "}\n";
 }
 
-void SILDebugScope::dump(SILModule &Mod) const {
+void SILDebugScope::print(SILModule &Mod) const {
   // We just use the default indent and llvm::errs().
-  dump(Mod.getASTContext().SourceMgr);
+  print(Mod.getASTContext().SourceMgr);
 }
 
 #if SWIFT_COMPILER_IS_MSVC

lib/SILGen/Initialization.h

@@ -148,6 +148,9 @@ class Initialization {
                                    ManagedValue explodedElement,
                                    bool isInit) = 0;
 
+  /// Whether to emit a debug value during initialization.
+  void setEmitDebugValueOnInit(bool emit) { EmitDebugValueOnInit = emit; }
+
   /// Perform post-initialization bookkeeping for this initialization.
   virtual void finishInitialization(SILGenFunction &SGF) {}
 
@@ -158,6 +161,9 @@ class Initialization {
                      "uninitialized");
   }
 
+protected:
+  bool EmitDebugValueOnInit = true;
+
 private:
   Initialization(const Initialization &) = delete;
   Initialization(Initialization &&) = delete;

lib/SILGen/SILGenDecl.cpp

@@ -555,14 +555,13 @@ class LetValueInitialization : public Initialization {
     SGF.VarLocs[vd] = SILGenFunction::VarLoc::get(value);
 
     // Emit a debug_value[_addr] instruction to record the start of this value's
-    // lifetime.
+    // lifetime, if permitted to do so.
+    if (!EmitDebugValueOnInit)
+      return;
     SILLocation PrologueLoc(vd);
     PrologueLoc.markAsPrologue();
     SILDebugVariable DbgVar(vd->isLet(), /*ArgNo=*/0);
-    if (address)
-      SGF.B.createDebugValueAddr(PrologueLoc, value, DbgVar);
-    else
-      SGF.B.createDebugValue(PrologueLoc, value, DbgVar);
+    SGF.B.emitDebugDescription(PrologueLoc, value, DbgVar);
   }
 
   void copyOrInitValueInto(SILGenFunction &SGF, SILLocation loc,

lib/SILGen/SILGenFunction.h

@@ -324,7 +324,7 @@ class LLVM_LIBRARY_VISIBILITY SILGenFunction
   std::vector<BreakContinueDest> BreakContinueDestStack;
   std::vector<PatternMatchContext*> SwitchStack;
   /// Keep track of our current nested scope.
-  std::vector<SILDebugScope*> DebugScopeStack;
+  std::vector<const SILDebugScope *> DebugScopeStack;
 
   /// The cleanup depth and BB for when the operand of a
   /// BindOptionalExpr is a missing value.
@@ -571,12 +571,15 @@ class LLVM_LIBRARY_VISIBILITY SILGenFunction
   StringRef getMagicFilePathString(SourceLoc loc);
   StringRef getMagicFunctionString();
 
-  /// Push a new debug scope and set its parent pointer.
+  /// Enter the debug scope for \p Loc, creating it if necessary.
   void enterDebugScope(SILLocation Loc) {
     auto *Parent =
         DebugScopeStack.size() ? DebugScopeStack.back() : F.getDebugScope();
-    auto *DS = new (SGM.M)
-        SILDebugScope(Loc.getAsRegularLocation(), &getFunction(), Parent);
+    auto *DS = Parent;
+    // Don't nest a scope for Loc under Parent unless it's actually different.
+    if (Parent->getLoc().getAsRegularLocation() != Loc.getAsRegularLocation())
+      DS = DS = new (SGM.M)
+          SILDebugScope(Loc.getAsRegularLocation(), &getFunction(), Parent);
     DebugScopeStack.push_back(DS);
     B.setCurrentDebugScope(DS);
   }

lib/SILGen/SILGenPattern.cpp

@@ -1193,6 +1193,17 @@ void PatternMatchEmission::bindVariable(Pattern *pattern, VarDecl *var,
 
   // Initialize the variable value.
   InitializationPtr init = SGF.emitInitializationForVarDecl(var, immutable);
+
+  // Do not emit debug descriptions at this stage.
+  //
+  // If there are multiple let bindings, the value is forwarded to the case
+  // block via a phi. Emitting duplicate debug values for the incoming values
+  // leads to bogus debug info -- we must emit the debug value only on the phi.
+  //
+  // If there's only one let binding, we still want to wait until we can nest
+  // the scope for the case body under the scope for the pattern match.
+  init->setEmitDebugValueOnInit(false);
+
   auto mv = value.getFinalManagedValue();
   if (shouldTake(value, isIrrefutable)) {
     mv.forwardInto(SGF, pattern, init.get());
@@ -2441,7 +2452,7 @@ void PatternMatchEmission::emitSharedCaseBlocks(
     // the order of variables that are the incoming BB arguments. Setup the
     // VarLocs to point to the incoming args and setup initialization so any
     // args needing Cleanup will get that as well.
-    Scope scope(SGF.Cleanups, CleanupLocation(caseBlock));
+    LexicalScope scope(SGF, CleanupLocation(caseBlock));
     unsigned argIndex = 0;
     for (auto *vd : caseBlock->getCaseBodyVariables()) {
       if (!vd->hasName())
@@ -2472,6 +2483,11 @@ void PatternMatchEmission::emitSharedCaseBlocks(
         mv = SGF.emitManagedRValueWithCleanup(arg);
       }
 
+      // Emit a debug description of the incoming arg, nested within the scope
+      // for the pattern match.
+      SILDebugVariable dbgVar(vd->isLet(), /*ArgNo=*/0);
+      SGF.B.emitDebugDescription(vd, mv.getValue(), dbgVar);
+
       if (vd->isLet()) {
         // Just emit a let and leave the cleanup alone.
         SGF.VarLocs[vd].value = mv.getValue();
@@ -2608,6 +2624,10 @@ static void switchCaseStmtSuccessCallback(SILGenFunction &SGF,
   // If we don't have a fallthrough or a multi-pattern 'case', we can emit the
   // body inline. Emit the statement here and bail early.
   if (!row.hasFallthroughTo() && caseBlock->getCaseLabelItems().size() == 1) {
+    // Debug values for case body variables must be nested within a scope for
+    // the case block to avoid name conflicts.
+    DebugScope scope(SGF, CleanupLocation(caseBlock));
+
     // If we have case body vars, set them up to point at the matching var
     // decls.
     if (caseBlock->hasCaseBodyVariables()) {
@@ -2628,6 +2648,11 @@ static void switchCaseStmtSuccessCallback(SILGenFunction &SGF,
           // Ok, we found a match. Update the VarLocs for the case block.
           auto v = SGF.VarLocs[vd];
           SGF.VarLocs[expected] = v;
+
+          // Emit a debug description for the variable, nested within a scope
+          // for the pattern match.
+          SILDebugVariable dbgVar(vd->isLet(), /*ArgNo=*/0);
+          SGF.B.emitDebugDescription(vd, v.value, dbgVar);
         }
       }
     }
@@ -2747,7 +2772,7 @@ void SILGenFunction::emitSwitchStmt(SwitchStmt *S) {
   emitProfilerIncrement(S);
   JumpDest contDest(contBB, Cleanups.getCleanupsDepth(), CleanupLocation(S));
 
-  Scope switchScope(Cleanups, CleanupLocation(S));
+  LexicalScope switchScope(*this, CleanupLocation(S));
 
   // Enter a break/continue scope.  If we wanted a continue
   // destination, it would probably be out here.

test/AutoDiff/SILOptimizer/activity_analysis.swift

@@ -827,10 +827,10 @@ func testActiveEnumAddr<T>(_ e: IndirectEnum<T>) -> T {
 // CHECK: [ACTIVE]   %6 = unchecked_take_enum_data_addr %3 : $*IndirectEnum<T>, #IndirectEnum.case1!enumelt
 // CHECK: [ACTIVE]   %7 = alloc_stack $T, let, name "y1"
 // CHECK: bb2:
-// CHECK: [ACTIVE]   %14 = unchecked_take_enum_data_addr %3 : $*IndirectEnum<T>, #IndirectEnum.case2!enumelt
-// CHECK: [ACTIVE]   %15 = tuple_element_addr %14 : $*(Float, T), 0
-// CHECK: [VARIED]   %16 = load [trivial] %15 : $*Float
-// CHECK: [ACTIVE]   %17 = tuple_element_addr %14 : $*(Float, T), 1
-// CHECK: [ACTIVE]   %18 = alloc_stack $T, let, name "y2"
+// CHECK: [ACTIVE] {{.*}} = unchecked_take_enum_data_addr {{.*}} : $*IndirectEnum<T>, #IndirectEnum.case2!enumelt
+// CHECK: [ACTIVE] {{.*}} = tuple_element_addr {{.*}} : $*(Float, T), 0
+// CHECK: [VARIED] {{.*}} = load [trivial] {{.*}} : $*Float
+// CHECK: [ACTIVE] {{.*}} = tuple_element_addr {{.*}} : $*(Float, T), 1
+// CHECK: [ACTIVE] {{.*}} = alloc_stack $T, let, name "y2"
 // CHECK: bb3:
-// CHECK: [NONE]   %25 = tuple ()
+// CHECK: [NONE]   {{.*}} = tuple ()