[Parse] Discriminate local variables

include/swift/AST/Decl.h

@@ -2150,6 +2150,7 @@ class ValueDecl : public Decl {
   DeclName Name;
   SourceLoc NameLoc;
   llvm::PointerIntPair<Type, 3, OptionalEnum<AccessLevel>> TypeAndAccess;
+  unsigned LocalDiscriminator = 0;
 
 private:
     bool isUsableFromInline() const;

include/swift/Parse/Parser.h

@@ -761,6 +761,7 @@ class Parser {
   ParserStatus parseLineDirective(bool isLine = false);
 
   void setLocalDiscriminator(ValueDecl *D);
+  void setLocalDiscriminatorToParamList(ParameterList *PL);
 
   /// Parse the optional attributes before a declaration.
   bool parseDeclAttributeList(DeclAttributes &Attributes,

include/swift/Serialization/ModuleFormat.h

@@ -55,7 +55,7 @@ const uint16_t VERSION_MAJOR = 0;
 /// describe what change you made. The content of this comment isn't important;
 /// it just ensures a conflict if two people change the module format.
 /// Don't worry about adhering to the 80-column limit for this line.
-const uint16_t VERSION_MINOR = 418; // Last change: add begin_access [builtin].
+const uint16_t VERSION_MINOR = 419; // Last change: Remove discriminator from LocalDeclTableInfo.
 
 using DeclIDField = BCFixed<31>;
 

lib/AST/ASTContext.cpp

@@ -226,10 +226,6 @@ FOR_KNOWN_FOUNDATION_TYPES(CACHE_FOUNDATION_DECL)
   /// \brief Map from Swift declarations to brief comments.
   llvm::DenseMap<const Decl *, StringRef> BriefComments;
 
-  /// \brief Map from local declarations to their discriminators.
-  /// Missing entries implicitly have value 0.
-  llvm::DenseMap<const ValueDecl *, unsigned> LocalDiscriminators;
-
   /// \brief Map from declarations to foreign error conventions.
   /// This applies to both actual imported functions and to @objc functions.
   llvm::DenseMap<const AbstractFunctionDecl *,
@@ -1796,24 +1792,6 @@ void ASTContext::setBriefComment(const Decl *D, StringRef Comment) {
   getImpl().BriefComments[D] = Comment;
 }
 
-unsigned ValueDecl::getLocalDiscriminator() const {
-  assert(getDeclContext()->isLocalContext());
-  auto &discriminators = getASTContext().getImpl().LocalDiscriminators;
-  auto it = discriminators.find(this);
-  if (it == discriminators.end())
-    return 0;
-  return it->second;
-}
-
-void ValueDecl::setLocalDiscriminator(unsigned index) {
-  assert(getDeclContext()->isLocalContext());
-  if (!index) {
-    assert(!getASTContext().getImpl().LocalDiscriminators.count(this));
-    return;
-  }
-  getASTContext().getImpl().LocalDiscriminators.insert({this, index});
-}
-
 NormalProtocolConformance *
 ASTContext::getBehaviorConformance(Type conformingType,
                                    ProtocolDecl *protocol,
@@ -2034,7 +2012,6 @@ size_t ASTContext::getTotalMemory() const {
     llvm::capacity_in_bytes(getImpl().ModuleLoaders) +
     llvm::capacity_in_bytes(getImpl().RawComments) +
     llvm::capacity_in_bytes(getImpl().BriefComments) +
-    llvm::capacity_in_bytes(getImpl().LocalDiscriminators) +
     llvm::capacity_in_bytes(getImpl().ModuleTypes) +
     llvm::capacity_in_bytes(getImpl().GenericParamTypes) +
     // getImpl().GenericFunctionTypes ?

lib/AST/Decl.cpp

@@ -1680,6 +1680,16 @@ bool ValueDecl::needsCapture() const {
   return !isa<TypeDecl>(this);
 }
 
+unsigned ValueDecl::getLocalDiscriminator() const {
+  return LocalDiscriminator;
+}
+
+void ValueDecl::setLocalDiscriminator(unsigned index) {
+  assert(getDeclContext()->isLocalContext());
+  assert(LocalDiscriminator == 0 && "LocalDiscriminator is set multiple times");
+  LocalDiscriminator = index;
+}
+
 ValueDecl *ValueDecl::getOverriddenDecl() const {
   if (auto fd = dyn_cast<FuncDecl>(this))
     return fd->getOverriddenDecl();

lib/IDE/SwiftSourceDocInfo.cpp

@@ -581,7 +581,7 @@ std::pair<bool, Pattern*> NameMatcher::walkToPatternPre(Pattern *P) {
   if (isDone() || shouldSkip(P->getSourceRange()))
     return std::make_pair(false, P);
 
-  tryResolve(ASTWalker::ParentTy(P), P->getLoc());
+  tryResolve(ASTWalker::ParentTy(P), P->getStartLoc());
   return std::make_pair(!isDone(), P);
 }
 

lib/Parse/ParseDecl.cpp

@@ -2409,6 +2409,14 @@ void Parser::setLocalDiscriminator(ValueDecl *D) {
   D->setLocalDiscriminator(discriminator);
 }
 
+void Parser::setLocalDiscriminatorToParamList(ParameterList *PL) {
+  for (auto P : *PL) {
+    if (!P->hasName() || P->isImplicit())
+      continue;
+    setLocalDiscriminator(P);
+  }
+}
+
 void Parser::delayParseFromBeginningToHere(ParserPosition BeginParserPosition,
                                            ParseDeclOptions Flags) {
   auto CurLoc = Tok.getLoc();
@@ -4282,6 +4290,8 @@ bool Parser::parseGetSetImpl(ParseDeclOptions Flags,
 
       // Establish the new context.
       ParseFunctionBody CC(*this, TheDecl);
+      for (auto PL : TheDecl->getParameterLists())
+        setLocalDiscriminatorToParamList(PL);
 
       // Parse the body.
       SmallVector<ASTNode, 16> Entries;
@@ -4373,6 +4383,8 @@ void Parser::parseAccessorBodyDelayed(AbstractFunctionDecl *AFD) {
   // Re-enter the lexical scope.
   Scope S(this, AccessorParserState->takeScope());
   ParseFunctionBody CC(*this, AFD);
+  for (auto PL : AFD->getParameterLists())
+    setLocalDiscriminatorToParamList(PL);
 
   SmallVector<ASTNode, 16> Entries;
   parseBraceItems(Entries);
@@ -4470,8 +4482,6 @@ VarDecl *Parser::parseDeclVarGetSet(Pattern *pattern,
   if (!PrimaryVar || !primaryVarIsWellFormed) {
     diagnose(pattern->getLoc(), diag::getset_nontrivial_pattern);
     Invalid = true;
-  } else {
-    setLocalDiscriminator(PrimaryVar);
   }
 
   TypeLoc TyLoc;
@@ -4881,6 +4891,7 @@ Parser::parseDeclVar(ParseDeclOptions Flags,
     pattern->forEachVariable([&](VarDecl *VD) {
       VD->setStatic(StaticLoc.isValid());
       VD->getAttrs() = Attributes;
+      setLocalDiscriminator(VD);
       Decls.push_back(VD);
     });
 
@@ -5326,6 +5337,8 @@ Parser::parseDeclFunc(SourceLoc StaticLoc, StaticSpellingKind StaticSpelling,
 
     // Establish the new context.
     ParseFunctionBody CC(*this, FD);
+    for (auto PL : FD->getParameterLists())
+      setLocalDiscriminatorToParamList(PL);
 
     // Check to see if we have a "{" to start a brace statement.
     if (Tok.is(tok::l_brace)) {
@@ -5396,6 +5409,8 @@ bool Parser::parseAbstractFunctionBodyDelayed(AbstractFunctionDecl *AFD) {
   // Re-enter the lexical scope.
   Scope S(this, FunctionParserState->takeScope());
   ParseFunctionBody CC(*this, AFD);
+  for (auto PL : AFD->getParameterLists())
+    setLocalDiscriminatorToParamList(PL);
 
   ParserResult<BraceStmt> Body =
       parseBraceItemList(diag::func_decl_without_brace);
@@ -6258,6 +6273,8 @@ Parser::parseDeclInit(ParseDeclOptions Flags, DeclAttributes &Attributes) {
     } else {
       // Parse the body.
       ParseFunctionBody CC(*this, CD);
+      for (auto PL : CD->getParameterLists())
+        setLocalDiscriminatorToParamList(PL);
 
       if (!isDelayedParsingEnabled()) {
         if (Context.Stats)

lib/Parse/ParseExpr.cpp

@@ -2731,6 +2731,7 @@ ParserResult<Expr> Parser::parseExprClosure() {
   if (params) {
     // Add the parameters into scope.
     addParametersToScope(params);
+    setLocalDiscriminatorToParamList(params);
   } else {
     // There are no parameters; allow anonymous closure variables.
     // FIXME: We could do this all the time, and then provide Fix-Its

lib/Parse/ParseStmt.cpp

@@ -975,6 +975,7 @@ static void parseGuardedPattern(Parser &P, GuardedPattern &result,
     // represents tuples and var patterns as tupleexprs and
     // unresolved_pattern_expr nodes, instead of as proper pattern nodes.
     patternResult.get()->forEachVariable([&](VarDecl *VD) {
+      P.setLocalDiscriminator(VD);
       if (VD->hasName()) P.addToScope(VD);
       boundDecls.push_back(VD);
     });
@@ -996,6 +997,8 @@ static void parseGuardedPattern(Parser &P, GuardedPattern &result,
       for (auto previous : boundDecls) {
         if (previous->hasName() && previous->getName() == VD->getName()) {
           found = true;
+          // Use the same local discriminator.
+          VD->setLocalDiscriminator(previous->getLocalDiscriminator());
           break;
         }
       }
@@ -1397,6 +1400,7 @@ Parser::parseStmtConditionElement(SmallVectorImpl<StmtConditionElement> &result,
   // Add variable bindings from the pattern to our current scope and mark
   // them as being having a non-pattern-binding initializer.
   ThePattern.get()->forEachVariable([&](VarDecl *VD) {
+    setLocalDiscriminator(VD);
     if (VD->hasName())
       addToScope(VD);
     VD->setHasNonPatternBindingInit();

lib/Serialization/ModuleFile.cpp

@@ -429,7 +429,7 @@ class ModuleFile::LocalDeclTableInfo {
 public:
   using internal_key_type = StringRef;
   using external_key_type = internal_key_type;
-  using data_type = std::pair<DeclID, unsigned>; // ID, local discriminator
+  using data_type = DeclID;
   using hash_value_type = uint32_t;
   using offset_type = unsigned;
 
@@ -447,7 +447,7 @@ class ModuleFile::LocalDeclTableInfo {
 
   static std::pair<unsigned, unsigned> ReadKeyDataLength(const uint8_t *&data) {
     unsigned keyLength = endian::readNext<uint16_t, little, unaligned>(data);
-    return { keyLength, sizeof(uint32_t) + sizeof(unsigned) };
+    return { keyLength, sizeof(uint32_t) };
   }
 
   static internal_key_type ReadKey(const uint8_t *data, unsigned length) {
@@ -456,9 +456,7 @@ class ModuleFile::LocalDeclTableInfo {
 
   static data_type ReadData(internal_key_type key, const uint8_t *data,
                             unsigned length) {
-    auto declID = endian::readNext<uint32_t, little, unaligned>(data);
-    auto discriminator = endian::readNext<unsigned, little, unaligned>(data);
-    return { declID, discriminator };
+    return endian::readNext<uint32_t, little, unaligned>(data);
   }
 };
 
@@ -1516,7 +1514,7 @@ TypeDecl *ModuleFile::lookupLocalType(StringRef MangledName) {
   if (iter == LocalTypeDecls->end())
     return nullptr;
 
-  return cast<TypeDecl>(getDecl((*iter).first));
+  return cast<TypeDecl>(getDecl(*iter));
 }
 
 TypeDecl *ModuleFile::lookupNestedType(Identifier name,
@@ -2021,10 +2019,8 @@ ModuleFile::getLocalTypeDecls(SmallVectorImpl<TypeDecl *> &results) {
   if (!LocalTypeDecls)
     return;
 
-  for (auto entry : LocalTypeDecls->data()) {
-    auto DeclID = entry.first;
+  for (auto DeclID : LocalTypeDecls->data()) {
     auto TD = cast<TypeDecl>(getDecl(DeclID));
-    TD->setLocalDiscriminator(entry.second);
     results.push_back(TD);
   }
 }

lib/Serialization/Serialization.cpp

@@ -228,7 +228,7 @@ namespace {
   public:
     using key_type = std::string;
     using key_type_ref = StringRef;
-    using data_type = std::pair<DeclID, unsigned>; // ID, local discriminator
+    using data_type = DeclID;
     using data_type_ref = const data_type &;
     using hash_value_type = uint32_t;
     using offset_type = unsigned;
@@ -242,7 +242,7 @@ namespace {
                                                     key_type_ref key,
                                                     data_type_ref data) {
       uint32_t keyLength = key.size();
-      uint32_t dataLength = sizeof(uint32_t) + sizeof(unsigned);
+      uint32_t dataLength = sizeof(uint32_t);
       endian::Writer<little> writer(out);
       writer.write<uint16_t>(keyLength);
       return { keyLength, dataLength };
@@ -256,8 +256,7 @@ namespace {
                   unsigned len) {
       static_assert(declIDFitsIn32Bits(), "DeclID too large");
       endian::Writer<little> writer(out);
-      writer.write<uint32_t>(data.first);
-      writer.write<unsigned>(data.second);
+      writer.write<uint32_t>(data);
     }
   };
 
@@ -4909,9 +4908,7 @@ void Serializer::writeAST(ModuleOrSourceFile DC,
       Mangle::ASTMangler Mangler;
       std::string MangledName = Mangler.mangleDeclAsUSR(TD, /*USRPrefix*/"");
       assert(!MangledName.empty() && "Mangled type came back empty!");
-      localTypeGenerator.insert(MangledName, {
-        addDeclRef(TD), TD->getLocalDiscriminator()
-      });
+      localTypeGenerator.insert(MangledName, addDeclRef(TD));
 
       if (auto IDC = dyn_cast<IterableDeclContext>(TD)) {
         collectInterestingNestedDeclarations(*this, IDC->getMembers(),

test/refactoring/rename/Outputs/local/casebind_1.swift.expected

@@ -0,0 +1,46 @@
+func test1() {
+  if true {
+    let x = 1
+    print(x)
+  } else {
+    let x = 2
+    print(x)
+  }
+}
+
+func test2(arg1: Int?, arg2: (Int, String)?) {
+  if let x = arg1 {
+    print(x)
+  } else if let (x, y) = arg2 {
+    print(x, y)
+  }
+}
+
+func test3(arg: Int?) {
+  switch arg {
+  case let .some(xRenamed) where xRenamed == 0:
+    print(xRenamed)
+  case let .some(x) where x == 1,
+       let .some(x) where x == 2: // FIXME: This 'x' in '.some(x)' isn't properly renamed in 'casebind_2' case.
+    print(x)
+  default:
+    break
+  }
+}
+
+struct Err1 : Error { }
+func test4(arg: () throws -> Void) {
+  do {
+    try arg()
+  } catch let x as Err1 {
+    print(x)
+  } catch let x {
+    print(x)
+  }
+}
+
+func test5(_ x: Int) {
+  let x = x 
+  print(x)
+}
+

test/refactoring/rename/Outputs/local/casebind_2.swift.expected

@@ -0,0 +1,46 @@
+func test1() {
+  if true {
+    let x = 1
+    print(x)
+  } else {
+    let x = 2
+    print(x)
+  }
+}
+
+func test2(arg1: Int?, arg2: (Int, String)?) {
+  if let x = arg1 {
+    print(x)
+  } else if let (x, y) = arg2 {
+    print(x, y)
+  }
+}
+
+func test3(arg: Int?) {
+  switch arg {
+  case let .some(x) where x == 0:
+    print(x)
+  case let .some(xRenamed) where xRenamed == 1,
+       let .some(x) where xRenamed == 2: // FIXME: This 'x' in '.some(x)' isn't properly renamed in 'casebind_2' case.
+    print(xRenamed)
+  default:
+    break
+  }
+}
+
+struct Err1 : Error { }
+func test4(arg: () throws -> Void) {
+  do {
+    try arg()
+  } catch let x as Err1 {
+    print(x)
+  } catch let x {
+    print(x)
+  }
+}
+
+func test5(_ x: Int) {
+  let x = x 
+  print(x)
+}
+