Merge pull request #34125 from slavapestov/local-redeclaration-checking

Implement re-declaration checking for declarations in local context

Author: slavapestov

include/swift/AST/NameLookup.h

@@ -396,31 +396,6 @@ class VectorDeclConsumer : public VisibleDeclConsumer {
   }
 };
 
-/// A consumer that inserts found decls with a matching name into an
-/// externally-owned SmallVector.
-class NamedDeclConsumer : public VisibleDeclConsumer {
-  virtual void anchor() override;
-public:
-  DeclNameRef name;
-  SmallVectorImpl<LookupResultEntry> &results;
-  bool isTypeLookup;
-
-  NamedDeclConsumer(DeclNameRef name,
-                    SmallVectorImpl<LookupResultEntry> &results,
-                    bool isTypeLookup)
-    : name(name), results(results), isTypeLookup(isTypeLookup) {}
-
-  virtual void foundDecl(ValueDecl *VD, DeclVisibilityKind Reason,
-                         DynamicLookupInfo dynamicLookupInfo = {}) override {
-    // Give clients an opportunity to filter out non-type declarations early,
-    // to avoid circular validation.
-    if (isTypeLookup && !isa<TypeDecl>(VD))
-      return;
-    if (VD->getName().matchesRef(name.getFullName()))
-      results.push_back(LookupResultEntry(VD));
-  }
-};
-
 /// A consumer that filters out decls that are not accessible from a given
 /// DeclContext.
 class AccessFilteringDeclConsumer final : public VisibleDeclConsumer {
@@ -619,18 +594,33 @@ class AbstractASTScopeDeclConsumer {
   virtual ~AbstractASTScopeDeclConsumer() = default;
 
   /// Called for every ValueDecl visible from the lookup.
-  /// Returns true if the lookup can be stopped at this point.
-  /// BaseDC is per legacy
+  ///
   /// Takes an array in order to batch the consumption before setting
   /// IndexOfFirstOuterResult when necessary.
+  ///
+  /// \param baseDC either a type context or the local context of a
+  /// `self` parameter declaration. See LookupResult for a discussion
+  /// of type -vs- instance lookup results.
+  ///
+  /// \return true if the lookup should be stopped at this point.
   virtual bool consume(ArrayRef<ValueDecl *> values, DeclVisibilityKind vis,
                        NullablePtr<DeclContext> baseDC = nullptr) = 0;
 
-  /// Eventually this functionality should move into ASTScopeLookup
+  /// Look for members of a nominal type or extension scope.
+  ///
+  /// \return true if the lookup should be stopped at this point.
   virtual bool
   lookInMembers(DeclContext *const scopeDC,
                 NominalTypeDecl *const nominal) = 0;
 
+  /// Called right before looking at the parent scope of a BraceStmt.
+  ///
+  /// \return true if the lookup should be stopped at this point.
+  virtual bool
+  finishLookupInBraceStmt(BraceStmt *stmt) {
+    return false;
+  }
+
 #ifndef NDEBUG
   virtual void startingNextLookupStep() = 0;
   virtual void finishingLookup(std::string) const = 0;
@@ -686,7 +676,11 @@ class ASTScope {
 
   /// Lookup that only finds local declarations and does not trigger
   /// interface type computation.
+  ///
+  /// \param stopAfterInnermostBraceStmt If lookup should consider
+  /// local declarations inside the innermost syntactic scope only.
   static void lookupLocalDecls(SourceFile *, DeclName, SourceLoc,
+                               bool stopAfterInnermostBraceStmt,
                                SmallVectorImpl<ValueDecl *> &);
 
   /// Returns the result if there is exactly one, nullptr otherwise.

lib/AST/ASTScopeLookup.cpp

@@ -384,7 +384,13 @@ bool BraceStmtScope::lookupLocalsOrMembers(DeclConsumer consumer) const {
         localBindings.push_back(vd);
     }
   }
-  return consumer.consume(localBindings, DeclVisibilityKind::LocalVariable);
+  if (consumer.consume(localBindings, DeclVisibilityKind::LocalVariable))
+    return true;
+
+  if (consumer.finishLookupInBraceStmt(stmt))
+    return true;
+
+  return false;
 }
 
 bool PatternEntryInitializerScope::lookupLocalsOrMembers(

lib/AST/NameLookup.cpp

@@ -45,7 +45,6 @@ using namespace swift::namelookup;
 
 void VisibleDeclConsumer::anchor() {}
 void VectorDeclConsumer::anchor() {}
-void NamedDeclConsumer::anchor() {}
 
 ValueDecl *LookupResultEntry::getBaseDecl() const {
   if (BaseDC == nullptr)

lib/AST/UnqualifiedLookup.cpp

@@ -88,32 +88,6 @@ namespace {
                        SmallVectorImpl<LookupResultEntry> &results) const;
     };
 
-    enum class AddGenericParameters { Yes, No };
-
-#ifndef NDEBUG
-    /// A consumer for debugging that lets the UnqualifiedLookupFactory know when
-    /// finding something.
-    class InstrumentedNamedDeclConsumer : public NamedDeclConsumer {
-      virtual void anchor() override;
-      UnqualifiedLookupFactory *factory;
-      
-    public:
-      InstrumentedNamedDeclConsumer(UnqualifiedLookupFactory *factory,
-                                    DeclNameRef name,
-                                    SmallVectorImpl<LookupResultEntry> &results,
-                                    bool isTypeLookup)
-      : NamedDeclConsumer(name, results, isTypeLookup), factory(factory) {}
-      
-      virtual void foundDecl(ValueDecl *VD, DeclVisibilityKind Reason,
-                             DynamicLookupInfo dynamicLookupInfo = {}) override {
-        unsigned before = results.size();
-        NamedDeclConsumer::foundDecl(VD, Reason, dynamicLookupInfo);
-        unsigned after = results.size();
-        if (after > before)
-          factory->addedResult(results.back());
-      }
-    };
-#endif
     // Inputs
     const DeclNameRef Name;
     DeclContext *const DC;
@@ -128,12 +102,7 @@ namespace {
     const Options options;
     const bool isOriginallyTypeLookup;
     const NLOptions baseNLOptions;
-    // Transputs
-#ifndef NDEBUG
-    InstrumentedNamedDeclConsumer Consumer;
-#else
-    NamedDeclConsumer Consumer;
-#endif
+
     // Outputs
     SmallVectorImpl<LookupResultEntry> &Results;
     size_t &IndexOfFirstOuterResult;
@@ -279,11 +248,6 @@ UnqualifiedLookupFactory::UnqualifiedLookupFactory(
   options(options),
   isOriginallyTypeLookup(options.contains(Flags::TypeLookup)),
   baseNLOptions(computeBaseNLOptions(options, isOriginallyTypeLookup)),
-  #ifdef NDEBUG
-  Consumer(Name, Results, isOriginallyTypeLookup),
-  #else
-  Consumer(this, Name, Results, isOriginallyTypeLookup),
-  #endif
   Results(Results),
   IndexOfFirstOuterResult(IndexOfFirstOuterResult)
 {}
@@ -675,9 +639,6 @@ UnqualifiedLookupRequest::evaluate(Evaluator &evaluator,
 }
 
 #pragma mark debugging
-#ifndef NDEBUG
-void UnqualifiedLookupFactory::InstrumentedNamedDeclConsumer::anchor() {}
-#endif
 
 void UnqualifiedLookupFactory::ResultFinderForTypeContext::dump() const {
   (void)factory;
@@ -775,13 +736,16 @@ namespace {
 
 class ASTScopeDeclConsumerForLocalLookup
     : public AbstractASTScopeDeclConsumer {
-  SmallVectorImpl<ValueDecl *> &results;
   DeclName name;
+  bool stopAfterInnermostBraceStmt;
+  SmallVectorImpl<ValueDecl *> &results;
 
 public:
   ASTScopeDeclConsumerForLocalLookup(
-      SmallVectorImpl<ValueDecl *> &results, DeclName name)
-    : results(results), name(name) {}
+      DeclName name, bool stopAfterInnermostBraceStmt,
+      SmallVectorImpl<ValueDecl *> &results)
+    : name(name), stopAfterInnermostBraceStmt(stopAfterInnermostBraceStmt),
+      results(results) {}
 
   bool consume(ArrayRef<ValueDecl *> values, DeclVisibilityKind vis,
                NullablePtr<DeclContext> baseDC) override {
@@ -792,14 +756,18 @@ class ASTScopeDeclConsumerForLocalLookup
       results.push_back(value);
     }
 
-    return !results.empty();
+    return (!stopAfterInnermostBraceStmt && !results.empty());
   }
 
   bool lookInMembers(DeclContext *const,
                      NominalTypeDecl *const) override {
     return true;
   }
 
+  bool finishLookupInBraceStmt(BraceStmt *stmt) override {
+    return stopAfterInnermostBraceStmt;
+  }
+
 #ifndef NDEBUG
   void startingNextLookupStep() override {}
   void finishingLookup(std::string) const override {}
@@ -812,15 +780,19 @@ class ASTScopeDeclConsumerForLocalLookup
 /// Lookup that only finds local declarations and does not trigger
 /// interface type computation.
 void ASTScope::lookupLocalDecls(SourceFile *sf, DeclName name, SourceLoc loc,
+                                bool stopAfterInnermostBraceStmt,
                                 SmallVectorImpl<ValueDecl *> &results) {
-  ASTScopeDeclConsumerForLocalLookup consumer(results, name);
+  ASTScopeDeclConsumerForLocalLookup consumer(name, stopAfterInnermostBraceStmt,
+                                              results);
   ASTScope::unqualifiedLookup(sf, loc, consumer);
 }
 
 ValueDecl *ASTScope::lookupSingleLocalDecl(SourceFile *sf, DeclName name,
                                            SourceLoc loc) {
   SmallVector<ValueDecl *, 1> result;
-  ASTScope::lookupLocalDecls(sf, name, loc, result);
+  ASTScope::lookupLocalDecls(sf, name, loc,
+                             /*finishLookupInBraceStmt=*/false,
+                             result);
   if (result.size() != 1)
     return nullptr;
   return result[0];

lib/Sema/TypeCheckAttr.cpp

@@ -3206,12 +3206,6 @@ void AttributeChecker::visitNonEphemeralAttr(NonEphemeralAttr *attr) {
   attr->setInvalid();
 }
 
-void TypeChecker::checkParameterAttributes(ParameterList *params) {
-  for (auto param: *params) {
-    checkDeclAttributes(param);
-  }
-}
-
 void AttributeChecker::checkOriginalDefinedInAttrs(Decl *D,
     ArrayRef<OriginallyDefinedInAttr*> Attrs) {
   if (Attrs.empty())