Drop references to name binding as a phase

CodaFi · hamishknight · commit 92c8a65f09ad · 2020-03-29T18:51:09.000-07:00
A lot of places appear to mean "name lookup".  A few places meant "import resolution".
diff --git a/docs/CompilerPerformance.md b/docs/CompilerPerformance.md
@@ -570,7 +570,7 @@ compilers on hand while you're working.
     Total Execution Time: 0.0876 seconds (0.0877 wall clock)
 
      ---User Time---   --System Time--   --User+System--   ---Wall Time---  --- Name ---
-     0.0241 ( 53.9%)   0.0394 ( 92.0%)   0.0635 ( 72.5%)   0.0635 ( 72.5%)  Name binding
+     0.0241 ( 53.9%)   0.0394 ( 92.0%)   0.0635 ( 72.5%)   0.0635 ( 72.5%)  Import resolution
      0.0170 ( 38.0%)   0.0025 (  5.8%)   0.0195 ( 22.3%)   0.0195 ( 22.2%)  Type checking / Semantic analysis
      0.0013 (  3.0%)   0.0004 (  0.8%)   0.0017 (  1.9%)   0.0017 (  1.9%)  LLVM output
      0.0010 (  2.3%)   0.0003 (  0.7%)   0.0013 (  1.5%)   0.0013 (  1.5%)  SILGen
diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def
@@ -661,7 +661,7 @@ NOTE(add_return_type_note,none,
      "did you mean to add a return type?", ())
 
 //------------------------------------------------------------------------------
-// MARK: Name Binding
+// MARK: Import Resolution
 //------------------------------------------------------------------------------
 
 ERROR(sema_no_import,Fatal,
diff --git a/include/swift/AST/Expr.h b/include/swift/AST/Expr.h
@@ -1333,9 +1333,9 @@ class SuperRefExpr : public Expr {
   }
 };
 
-/// A reference to a type in expression context, spelled out as a TypeLoc. Sema
-/// forms this expression as a result of name binding.  This always has
-/// MetaTypetype.
+/// A reference to a type in expression context, spelled out as a TypeLoc.
+///
+/// The type of this expression is always \c MetaTypeType.
 class TypeExpr : public Expr {
   TypeLoc Info;
   TypeExpr(Type Ty);
@@ -4831,7 +4831,7 @@ class AssignExpr : public Expr {
 };
 
 /// A pattern production that has been parsed but hasn't been resolved
-/// into a complete pattern. Name binding converts these into standalone pattern
+/// into a complete pattern. Pattern checking converts these into standalone pattern
 /// nodes or raises an error if a pattern production appears in an invalid
 /// position.
 class UnresolvedPatternExpr : public Expr {
diff --git a/include/swift/AST/ImportCache.h b/include/swift/AST/ImportCache.h
@@ -148,7 +148,7 @@ class alignas(ModuleDecl::ImportedModule) ImportCache {
                                  const DeclContext *dc);
 
   /// This is a hack to cope with main file parsing and REPL parsing, where
-  /// we can add ImportDecls after name binding.
+  /// we can add ImportDecls after import resolution.
   void clear() {
     ImportSetForDC.clear();
   }
diff --git a/include/swift/AST/SourceFile.h b/include/swift/AST/SourceFile.h
@@ -23,9 +23,8 @@ class PersistentParserState;
 /// A file containing Swift source code.
 ///
 /// This is a .swift or .sil file (or a virtual file, such as the contents of
-/// the REPL). Since it contains raw source, it must be parsed and name-bound
-/// before being used for anything; a full type-check is also necessary for
-/// IR generation.
+/// the REPL). Since it contains raw source, it must be type checked for IR
+/// generation.
 class SourceFile final : public FileUnit {
   friend class ParseSourceFileRequest;
 
@@ -128,7 +127,7 @@ class SourceFile final : public FileUnit {
 
   /// This is the list of modules that are imported by this module.
   ///
-  /// This is filled in by the Name Binding phase.
+  /// This is filled in by the import resolution phase.
   ArrayRef<ImportedModuleDesc> Imports;
 
   /// A unique identifier representing this file; used to mark private decls
@@ -316,7 +315,7 @@ class SourceFile final : public FileUnit {
   const SourceFileKind Kind;
 
   enum ASTStage_t {
-    /// The source file is not name bound or type checked.
+    /// The source file has not had its imports resolved or been type checked.
     Unprocessed,
     /// Import resolution has completed.
     ImportsResolved,
diff --git a/include/swift/AST/TypeRepr.h b/include/swift/AST/TypeRepr.h
@@ -263,7 +263,7 @@ class ComponentIdentTypeRepr : public IdentTypeRepr {
   /// component.
   ///
   /// The initial parsed representation is always an identifier, and
-  /// name binding will resolve this to a specific declaration.
+  /// name lookup will resolve this to a specific declaration.
   llvm::PointerUnion<DeclNameRef, TypeDecl *> IdOrDecl;
 
   /// The declaration context from which the bound declaration was
@@ -282,7 +282,8 @@ class ComponentIdentTypeRepr : public IdentTypeRepr {
   /// correction.
   void overwriteNameRef(DeclNameRef newId) { IdOrDecl = newId; }
 
-  /// Return true if this has been name-bound already.
+  /// Return true if this name has been resolved to a type decl. This happens
+  /// during type resolution.
   bool isBound() const { return IdOrDecl.is<TypeDecl *>(); }
 
   TypeDecl *getBoundDecl() const { return IdOrDecl.dyn_cast<TypeDecl*>(); }
diff --git a/include/swift/AST/Types.h b/include/swift/AST/Types.h
@@ -1271,10 +1271,11 @@ class NominalOrBoundGenericNominalType : public AnyGenericType {
 };
 DEFINE_EMPTY_CAN_TYPE_WRAPPER(NominalOrBoundGenericNominalType, AnyGenericType)
 
-/// ErrorType - This represents a type that was erroneously constructed.  This
-/// is produced when parsing types and when name binding type aliases, and is
-/// installed in declaration that use these erroneous types.  All uses of a
-/// declaration of invalid type should be ignored and not re-diagnosed.
+/// ErrorType - Represents the type of an erroneously constructed declaration,
+/// expression, or type. When creating ErrorTypes, an associated error
+/// diagnostic should always be emitted. That way when later stages of
+/// compilation encounter an ErrorType installed by earlier phases they do not
+/// have to emit further diagnostics to abort compilation.
 class ErrorType final : public TypeBase {
   friend class ASTContext;
   // The Error type is always canonical.
@@ -6410,11 +6411,11 @@ inline ArrayRef<AnyFunctionType::Param> AnyFunctionType::getParams() const {
     llvm_unreachable("Undefined function type");
   }
 }
-  
+
 /// If this is a method in a type or extension thereof, compute
 /// and return a parameter to be used for the 'self' argument.  The type of
 /// the parameter is the empty Type() if no 'self' argument should exist. This
-/// can only be used after name binding has resolved types.
+/// can only be used after types have been resolved.
 ///
 /// \param isInitializingCtor Specifies whether we're computing the 'self'
 /// type of an initializing constructor, which accepts an instance 'self'
diff --git a/include/swift/Frontend/Frontend.h b/include/swift/Frontend/Frontend.h
@@ -623,7 +623,7 @@ class CompilerInstance {
   void performParseOnly(bool EvaluateConditionals = false,
                         bool CanDelayBodies = true);
 
-  /// Parses and performs name binding on all input files.
+  /// Parses and performs import resolution on all input files.
   ///
   /// This is similar to a parse-only invocation, but module imports will also
   /// be processed.
diff --git a/include/swift/Parse/Scope.h b/include/swift/Parse/Scope.h
@@ -170,7 +170,7 @@ inline ValueDecl *ScopeInfo::lookupValueName(DeclNameRef Name) {
   assert(CurScope && "no scope");
   // If we found nothing, or we found a decl at the top-level, return nothing.
   // We ignore results at the top-level because we may have overloading that
-  // will be resolved properly by name binding.
+  // will be resolved properly by name lookup.
   std::pair<unsigned, ValueDecl *> Res = HT.lookup(CurScope->HTScope,
                                                    Name.getFullName());
   if (Res.first < ResolvableDepth)
diff --git a/lib/AST/Module.cpp b/lib/AST/Module.cpp
@@ -1351,14 +1351,14 @@ void ModuleDecl::getImportedModules(SmallVectorImpl<ImportedModule> &modules,
 void
 SourceFile::getImportedModules(SmallVectorImpl<ModuleDecl::ImportedModule> &modules,
                                ModuleDecl::ImportFilter filter) const {
-  // FIXME: Ideally we should assert that the file has been name bound before
-  // calling this function. However unfortunately that can cause issues for
-  // overlays which can depend on a Clang submodule for the underlying framework
-  // they are overlaying, which causes us to attempt to load the overlay again.
-  // We need to find a way to ensure that an overlay dependency with the same
-  // name as the overlay always loads the underlying Clang module. We currently
-  // handle this for a direct import from the overlay, but not when it happens
-  // through other imports.
+  // FIXME: Ideally we should assert that the file has had its imports resolved
+  // before calling this function. However unfortunately that can cause issues
+  // for overlays which can depend on a Clang submodule for the underlying
+  // framework they are overlaying, which causes us to attempt to load the
+  // overlay again. We need to find a way to ensure that an overlay dependency
+  // with the same name as the overlay always loads the underlying Clang module.
+  // We currently handle this for a direct import from the overlay, but not when
+  // it happens through other imports.
   assert(filter && "no imports requested?");
   for (auto desc : Imports) {
     ModuleDecl::ImportFilter requiredFilter;
diff --git a/lib/Parse/ParseExpr.cpp b/lib/Parse/ParseExpr.cpp
@@ -46,8 +46,8 @@ ParserResult<Expr> Parser::parseExprImpl(Diag<> Message,
   SyntaxParsingContext ExprParsingContext(SyntaxContext, SyntaxContextKind::Expr);
 
   // If we are parsing a refutable pattern, check to see if this is the start
-  // of a let/var/is pattern.  If so, parse it to an UnresolvedPatternExpr and
-  // name binding will perform final validation.
+  // of a let/var/is pattern.  If so, parse it as an UnresolvedPatternExpr and
+  // let pattern type checking determine its final form.
   //
   // Only do this if we're parsing a pattern, to improve QoI on malformed
   // expressions followed by (e.g.) let/var decls.
diff --git a/lib/ParseSIL/ParseSIL.cpp b/lib/ParseSIL/ParseSIL.cpp
@@ -1093,7 +1093,6 @@ static bool parseDeclSILOptional(bool *isTransparent,
 bool SILParser::performTypeLocChecking(TypeLoc &T, bool IsSILType,
                                        GenericEnvironment *GenericEnv,
                                        DeclContext *DC) {
-  // Do some type checking / name binding for the parsed type.
   if (GenericEnv == nullptr)
     GenericEnv = ContextGenericEnv;
 
diff --git a/lib/Sema/CSGen.cpp b/lib/Sema/CSGen.cpp
@@ -3135,8 +3135,8 @@ namespace {
     }
     
     Type visitUnresolvedPatternExpr(UnresolvedPatternExpr *expr) {
-      // If there are UnresolvedPatterns floating around after name binding,
-      // they are pattern productions in invalid positions. However, we will
+      // If there are UnresolvedPatterns floating around after pattern type
+      // checking, they are definitely invalid. However, we will
       // diagnose that condition elsewhere; to avoid unnecessary noise errors,
       // just plop an open type variable here.
       
diff --git a/lib/Sema/ConstraintSystem.h b/lib/Sema/ConstraintSystem.h
@@ -2323,8 +2323,8 @@ class ConstraintSystem {
   /// system, to avoid.
   ///
   /// FIXME: This caching should almost certainly be performed at the
-  /// module level, since type checking occurs after name binding,
-  /// and no new names are introduced after name binding.
+  /// module level, since type checking occurs after import resolution,
+  /// and no new names are introduced after that point.
   ///
   /// \returns A reference to the member-lookup result.
   LookupResult &lookupMember(Type base, DeclNameRef name);
diff --git a/lib/Sema/TypeCheckType.h b/lib/Sema/TypeCheckType.h
@@ -339,7 +339,7 @@ class TypeResolution {
 
   /// Resolves a TypeRepr to a type.
   ///
-  /// Performs name binding, checking of generic arguments, and so on in order
+  /// Performs name lookup, checking of generic arguments, and so on in order
   /// to create a well-formed type.
   ///
   /// \param TyR The type representation to check.
diff --git a/lib/Sema/TypeChecker.h b/lib/Sema/TypeChecker.h
@@ -423,7 +423,7 @@ Expr *resolveDeclRefExpr(UnresolvedDeclRefExpr *UDRE, DeclContext *Context);
 
 /// Validate the given type.
 ///
-/// Type validation performs name binding, checking of generic arguments,
+/// Type validation performs name lookup, checking of generic arguments,
 /// and so on to determine whether the given type is well-formed and can
 /// be used as a type.
 ///
diff --git a/test/NameBinding/library.swift b/test/NameBinding/library.swift
@@ -21,7 +21,7 @@ var c = 1
 var d = 2
 var e = 3
 
-// Name-binding with imports
+// Name lookup with imports.
 import imported_module
 func over1(_ x: UInt64) {} // expected-note{{found this candidate}}
 func over2(_ x: UInt32) {}
diff --git a/test/NameBinding/name-binding.swift b/test/NameBinding/name-binding.swift
@@ -23,7 +23,7 @@ var importedunion: unionSearchFlags = .backwards
 var notimported : MaybeInt // expected-error {{use of undeclared type 'MaybeInt'}}
 
 //===----------------------------------------------------------------------===//
-// Name binding stress test
+// Name lookup stress test
 //===----------------------------------------------------------------------===//
 
 var callee1 : () -> (Int,Int,Int)           // Takes nothing, returns tuple.
@@ -62,13 +62,12 @@ func test_varname_binding() {
 // ForwardIndex referencing of types.
 //===----------------------------------------------------------------------===//
 
-// We allow namebinding to look forward past a var declaration in the
-// main module
+// Lookup can find a decl declared later in the main module.
 var x : x_ty
 typealias x_ty = Int
 
-// We allow namebinding to look forward past a function declaration (and other
-// declarations which never have side-effects) in the main module
+// We allow name lookup to look forward past a function declaration (and other
+// declarations which never have side-effects) in the main module.
 func fy() -> y_ty { return 1 }
 typealias y_ty = Int
 
diff --git a/test/NameBinding/scope_map_lookup.swift b/test/NameBinding/scope_map_lookup.swift
@@ -1,7 +1,7 @@
 // XFAIL: *
 // RUN: %target-typecheck-verify-swift -enable-astscope-lookup
 
-// Name binding in default arguments
+// Name lookup in default arguments
 
 // FIXME: Semantic analysis should not recommend 'x' or 'y' here, because they
 // are not actually available.
@@ -12,7 +12,7 @@ func functionParamScopes(x: Int, y: Int = x) -> Int {
   return x + y
 }
 
-// Name binding in instance methods.
+// Name lookup in instance methods.
 class C1 {
 	var x = 0
 

Original file line number	Diff line number	Diff line change
`@@ -148,7 +148,7 @@ class alignas(ModuleDecl::ImportedModule) ImportCache {`
`148`	`148`	`const DeclContext *dc);`
`149`	`149`
`150`	`150`	`/// This is a hack to cope with main file parsing and REPL parsing, where`
`151`		`- /// we can add ImportDecls after name binding.`
	`151`	`+ /// we can add ImportDecls after import resolution.`
`152`	`152`	`void clear() {`
`153`	`153`	`ImportSetForDC.clear();`
`154`	`154`	`}`