Skip to content

NFC: Rename NameBinding to ImportResolution #30717

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Mar 30, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/CompilerPerformance.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion include/swift/AST/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
8 changes: 4 additions & 4 deletions include/swift/AST/Expr.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion include/swift/AST/ImportCache.h
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down
13 changes: 6 additions & 7 deletions include/swift/AST/SourceFile.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -316,10 +315,10 @@ 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,
/// Name binding has completed.
NameBound,
/// Import resolution has completed.
ImportsResolved,
/// Type checking has completed.
TypeChecked
};
Expand Down
5 changes: 3 additions & 2 deletions include/swift/AST/TypeRepr.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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*>(); }
Expand Down
13 changes: 7 additions & 6 deletions include/swift/AST/Types.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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'
Expand Down
2 changes: 1 addition & 1 deletion include/swift/Frontend/Frontend.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion include/swift/Parse/Scope.h
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
21 changes: 10 additions & 11 deletions include/swift/Subsystems.h
Original file line number Diff line number Diff line change
Expand Up @@ -118,33 +118,32 @@ namespace swift {
bool TokenizeInterpolatedString = true,
ArrayRef<Token> SplitTokens = ArrayRef<Token>());

/// Once parsing is complete, this walks the AST to resolve imports, record
/// operators, and do other top-level validation.
void performNameBinding(SourceFile &SF);
/// This walks the AST to resolve imports.
void performImportResolution(SourceFile &SF);

/// Once type-checking is complete, this instruments code with calls to an
/// intrinsic that record the expected values of local variables so they can
/// be compared against the results from the debugger.
void performDebuggerTestingTransform(SourceFile &SF);

/// Once parsing and name-binding are complete, this optionally transforms the
/// ASTs to add calls to external logging functions.
/// Once type checking is complete, this optionally transforms the ASTs to add
/// calls to external logging functions.
///
/// \param HighPerformance True if the playground transform should omit
/// instrumentation that has a high runtime performance impact.
void performPlaygroundTransform(SourceFile &SF, bool HighPerformance);

/// Once parsing and name-binding are complete this optionally walks the ASTs
/// to add calls to externally provided functions that simulate
/// "program counter"-like debugging events. See the comment at the top of
/// lib/Sema/PCMacro.cpp for a description of the calls inserted.
/// Once type checking is complete this optionally walks the ASTs to add calls
/// to externally provided functions that simulate "program counter"-like
/// debugging events. See the comment at the top of lib/Sema/PCMacro.cpp for a
/// description of the calls inserted.
void performPCMacro(SourceFile &SF);

/// Bind all 'extension' visible from \p SF to the extended nominal.
void bindExtensions(SourceFile &SF);

/// Once parsing and name-binding are complete, this walks the AST to resolve
/// types and diagnose problems therein.
/// Once import resolution is complete, this walks the AST to resolve types
/// and diagnose problems therein.
void performTypeChecking(SourceFile &SF);

/// Now that we have type-checked an entire module, perform any type
Expand Down
9 changes: 0 additions & 9 deletions lib/AST/ASTVerifier.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -419,10 +419,6 @@ class Verifier : public ASTWalker {
// Always verify the node as a parsed node.
verifyParsed(node);

// If we've bound names already, verify as a bound node.
if (!SF || SF->ASTStage >= SourceFile::NameBound)
verifyBound(node);

// If we've checked types already, do some extra verification.
if (!SF || SF->ASTStage >= SourceFile::TypeChecked) {
verifyCheckedAlways(node);
Expand Down Expand Up @@ -523,11 +519,6 @@ class Verifier : public ASTWalker {
verifyParsed(cast<typename ASTNodeBase<T>::BaseTy>(ASTNode));
}

void verifyBound(Expr *E) {}
void verifyBound(Stmt *S) {}
void verifyBound(Pattern *P) {}
void verifyBound(Decl *D) {}

/// @{
/// These verification functions are always run on type checked ASTs
/// (even if there were errors).
Expand Down
18 changes: 9 additions & 9 deletions lib/AST/Module.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1182,7 +1182,7 @@ lookupOperatorDeclForName(const FileUnit &File, SourceLoc Loc,
}

auto &SF = cast<SourceFile>(File);
assert(SF.ASTStage >= SourceFile::NameBound);
assert(SF.ASTStage >= SourceFile::ImportsResolved);

// Check if the decl exists on the file.
if (auto *op = OperatorLookup<OP_DECL>::lookup(eval, desc))
Expand Down Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion lib/AST/UnqualifiedLookup.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1448,7 +1448,7 @@ bool UnqualifiedLookupFactory::shouldDiffer() const {
"swift/test/TypeCoercion/overload_noncall.swift",
"swift/test/expr/capture/nested_class.swift",
"swift/test/expr/capture/order.swift",
"swift/test/NameBinding/name-binding.swift"
"swift/test/NameLookup/name_lookup2.swift"
};
StringRef fileName = SF->getFilename();
return llvm::any_of(testsThatShouldDiffer, [&](const char *testFile) {
Expand Down
22 changes: 11 additions & 11 deletions lib/Frontend/Frontend.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -752,7 +752,7 @@ shouldImplicityImportSwiftOnoneSupportModule(CompilerInvocation &Invocation) {
}

void CompilerInstance::performParseAndResolveImportsOnly() {
performSemaUpTo(SourceFile::NameBound);
performSemaUpTo(SourceFile::ImportsResolved);
}

void CompilerInstance::performSema() {
Expand Down Expand Up @@ -926,12 +926,12 @@ void CompilerInstance::parseAndCheckTypesUpTo(
auto *SF = dyn_cast<SourceFile>(File);
if (!SF)
return true;
return SF->ASTStage >= SourceFile::NameBound;
return SF->ASTStage >= SourceFile::ImportsResolved;
}) && "some files have not yet had their imports resolved");
MainModule->setHasResolvedImports();

forEachFileToTypeCheck([&](SourceFile &SF) {
if (limitStage == SourceFile::NameBound) {
if (limitStage == SourceFile::ImportsResolved) {
bindExtensions(SF);
return;
}
Expand All @@ -951,8 +951,8 @@ void CompilerInstance::parseAndCheckTypesUpTo(
}
});

// If the limiting AST stage is name binding, we're done.
if (limitStage <= SourceFile::NameBound) {
// If the limiting AST stage is import resolution, we're done.
if (limitStage <= SourceFile::ImportsResolved) {
return;
}

Expand All @@ -967,8 +967,8 @@ void CompilerInstance::parseLibraryFile(
SourceFileKind::Library, implicitImports.kind, BufferID);
addAdditionalInitialImportsTo(NextInput, implicitImports);

// Name binding will lazily trigger parsing of the file.
performNameBinding(*NextInput);
// Import resolution will lazily trigger parsing of the file.
performImportResolution(*NextInput);
}

bool CompilerInstance::parsePartialModulesAndLibraryFiles(
Expand Down Expand Up @@ -997,7 +997,7 @@ bool CompilerInstance::parsePartialModulesAndLibraryFiles(

void CompilerInstance::parseAndTypeCheckMainFileUpTo(
SourceFile::ASTStage_t LimitStage) {
assert(LimitStage >= SourceFile::NameBound);
assert(LimitStage >= SourceFile::ImportsResolved);
FrontendStatsTracer tracer(getStatsReporter(),
"parse-and-typecheck-main-file");
bool mainIsPrimary =
Expand All @@ -1010,13 +1010,13 @@ void CompilerInstance::parseAndTypeCheckMainFileUpTo(
auto DidSuppressWarnings = Diags.getSuppressWarnings();
Diags.setSuppressWarnings(DidSuppressWarnings || !mainIsPrimary);

// For a primary, perform type checking if needed. Otherwise, just do name
// binding.
// For a primary, perform type checking if needed. Otherwise, just do import
// resolution.
if (mainIsPrimary && LimitStage >= SourceFile::TypeChecked) {
performTypeChecking(MainFile);
} else {
assert(!TheSILModule && "Should perform type checking for SIL");
performNameBinding(MainFile);
performImportResolution(MainFile);
}

// Parse the SIL decls if needed.
Expand Down
2 changes: 1 addition & 1 deletion lib/IDE/CompletionInstance.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,7 @@ bool CompletionInstance::performCachedOperaitonIfPossible(

// Re-process the whole file (parsing will be lazily triggered). Still
// re-use imported modules.
performNameBinding(*newSF);
performImportResolution(*newSF);
bindExtensions(*newSF);

#ifndef NDEBUG
Expand Down
4 changes: 2 additions & 2 deletions lib/Parse/ParseExpr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
1 change: 0 additions & 1 deletion lib/ParseSIL/ParseSIL.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
2 changes: 1 addition & 1 deletion lib/Sema/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,10 @@ add_swift_host_library(swiftSema STATIC
DerivedConformanceError.cpp
DerivedConformanceRawRepresentable.cpp
DerivedConformances.cpp
ImportResolution.cpp
InstrumenterSupport.cpp
LookupVisibleDecls.cpp
MiscDiagnostics.cpp
NameBinding.cpp
PCMacro.cpp
PlaygroundTransform.cpp
ResilienceDiagnostics.cpp
Expand Down
4 changes: 2 additions & 2 deletions lib/Sema/CSGen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
4 changes: 2 additions & 2 deletions lib/Sema/ConstraintSystem.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Loading