From 0f72b1c45e63e37605c9881243b4044bfc1bdbbb Mon Sep 17 00:00:00 2001 From: Nathaniel Roman Date: Tue, 12 Oct 2021 14:44:47 -0700 Subject: [PATCH] Package state in GlobalState (#4631) * Move PackageDB to GlobalState * Avoid re-processing all package files when package specs have not changed * remove TODOs * don't use unique_ptr in PackageDB::deepCopy * Apply suggestions from code review Co-authored-by: Jake Zimmerman Co-authored-by: Jake Zimmerman --- core/BUILD | 7 +- core/GlobalState.cc | 9 + core/GlobalState.h | 7 + core/packages/PackageDB.cc | 127 ++++++++++++++ core/packages/PackageDB.h | 49 ++++++ core/packages/PackageInfo.cc | 15 ++ core/packages/PackageInfo.h | 28 +++ main/lsp/LSPTypechecker.cc | 7 +- packager/packager.cc | 332 ++++++++++++++++------------------- packager/packager.h | 1 + 10 files changed, 399 insertions(+), 183 deletions(-) create mode 100644 core/packages/PackageDB.cc create mode 100644 core/packages/PackageDB.h create mode 100644 core/packages/PackageInfo.cc create mode 100644 core/packages/PackageInfo.h diff --git a/core/BUILD b/core/BUILD index 73f8eb921aa..776b07ebfca 100644 --- a/core/BUILD +++ b/core/BUILD @@ -6,6 +6,7 @@ cc_library( "*.h", "types/*.cc", "lsp/*.cc", + "packages/*.cc", ], exclude = [ # workaround https://github.com/flycheck/flycheck/issues/248 in emacs @@ -18,7 +19,11 @@ cc_library( hdrs = [ "GlobalSubstitution.h", "core.h", - ] + glob(["errors/*.h"]) + glob(["lsp/*.h"]), + ] + glob([ + "errors/*.h", + "lsp/*.h", + "packages/*.h", + ]), linkstatic = select({ "//tools/config:linkshared": 0, "//conditions:default": 1, diff --git a/core/GlobalState.cc b/core/GlobalState.cc index babf2078ee1..0c3b70e47e8 100644 --- a/core/GlobalState.cc +++ b/core/GlobalState.cc @@ -1767,6 +1767,7 @@ unique_ptr GlobalState::deepCopy(bool keepId) const { for (auto &semanticExtension : this->semanticExtensions) { result->semanticExtensions.emplace_back(semanticExtension->deepCopy(*this, *result)); } + result->packageDB_ = packageDB_.deepCopy(); result->sanityCheck(); { Timer timeit2(tracer(), "GlobalState::deepCopyOut"); @@ -1903,6 +1904,14 @@ FileRef GlobalState::findFileByPath(string_view path) const { return FileRef(); } +const packages::PackageDB &GlobalState::packageDB() const { + return packageDB_; +} + +packages::UnfreezePackages GlobalState::unfreezePackages() { + return packageDB_.unfreeze(); +} + unique_ptr GlobalState::markFileAsTombStone(unique_ptr what, FileRef fref) { ENFORCE(fref.id() < what->filesUsed()); what->files[fref.id()]->sourceType = File::Type::TombStone; diff --git a/core/GlobalState.h b/core/GlobalState.h index e38eacca0fc..a48ac9429b9 100644 --- a/core/GlobalState.h +++ b/core/GlobalState.h @@ -8,6 +8,8 @@ #include "core/Names.h" #include "core/Symbols.h" #include "core/lsp/Query.h" +#include "core/packages/PackageDB.h" +#include "core/packages/PackageInfo.h" #include "main/pipeline/semantic_extension/SemanticExtension.h" #include @@ -150,6 +152,9 @@ class GlobalState final { static std::unique_ptr markFileAsTombStone(std::unique_ptr, FileRef fref); FileRef findFileByPath(std::string_view path) const; + const packages::PackageDB &packageDB() const; + packages::UnfreezePackages unfreezePackages(); + void mangleRenameSymbol(SymbolRef what, NameRef origName); spdlog::logger &tracer() const; unsigned int namesUsedTotal() const; @@ -297,6 +302,8 @@ class GlobalState final { UnorderedSet onlyErrorClasses; bool wasModified_ = false; + core::packages::PackageDB packageDB_; + bool freezeSymbolTable(); bool freezeNameTable(); bool freezeFileTable(); diff --git a/core/packages/PackageDB.cc b/core/packages/PackageDB.cc new file mode 100644 index 00000000000..b2fd3ef4814 --- /dev/null +++ b/core/packages/PackageDB.cc @@ -0,0 +1,127 @@ +#include "core/packages/PackageDB.h" +#include "core/Loc.h" + +using namespace std; + +namespace sorbet::core::packages { + +namespace { +class NonePackage final : public PackageInfo { +public: + core::NameRef mangledName() const { + return NameRef::noName(); + } + + const vector &pathPrefixes() const { + ENFORCE(false); + return prefixes; + } + + unique_ptr deepCopy() const { + ENFORCE(false); + return make_unique(); + } + + Loc definitionLoc() const { + ENFORCE(false); + return Loc::none(); + } + + ~NonePackage() {} + +private: + const vector prefixes; +}; +static const NonePackage NONE_PKG; +} // namespace + +UnfreezePackages::UnfreezePackages(PackageDB &db) : db(db) { + ENFORCE(db.frozen); + db.writerThread = this_thread::get_id(); + db.frozen = false; +} + +UnfreezePackages::~UnfreezePackages() { + ENFORCE(!db.frozen); + db.writerThread = std::thread::id(); + db.frozen = true; + // Note in the future we may want to change the data structures involved in a way that requires + // a finalization step. Controlling freeze/unfreeze with RAII gives a good hook to do that. +} + +NameRef PackageDB::enterPackage(unique_ptr pkg) { + ENFORCE(!frozen); + ENFORCE(writerThread == this_thread::get_id(), "PackageDB writes are not thread safe"); + auto nr = pkg->mangledName(); + auto prev = packages.find(nr); + if (prev == packages.end()) { + for (const auto &prefix : pkg->pathPrefixes()) { + packagesByPathPrefix[prefix] = nr; + } + } else { + // Package files do not have full featured content hashing. If the contents of one changes + // we always run slow-path and fully rebuild the set of packages. In some cases, the LSP + // fast-path may re-run on an unchanged package file. Sanity check to ensure the loc and + // prefixes are the same. + ENFORCE(prev->second->definitionLoc() == pkg->definitionLoc()); + ENFORCE(prev->second->pathPrefixes() == pkg->pathPrefixes()); + } + packages[nr] = move(pkg); + return nr; +} + +NameRef PackageDB::lookupPackage(NameRef pkgMangledName) const { + ENFORCE(pkgMangledName.exists()); + auto it = packages.find(pkgMangledName); + if (it == packages.end()) { + return NameRef::noName(); + } + return it->first; +} + +const PackageInfo &PackageDB::getPackageForFile(const core::GlobalState &gs, core::FileRef file) const { + ENFORCE(frozen); + auto &fileData = file.data(gs); + string_view path = fileData.path(); + int curPrefixPos = path.find_last_of('/'); + while (curPrefixPos != string::npos) { + const auto &it = packagesByPathPrefix.find(path.substr(0, curPrefixPos + 1)); + if (it != packagesByPathPrefix.end()) { + const auto &pkg = getPackageInfo(it->second); + ENFORCE(pkg.exists()); + return pkg; + } + + if (fileData.sourceType == core::File::Type::Package) { + // When looking up a `__package.rb` file do not search parent directories + break; + } + curPrefixPos = path.find_last_of('/', curPrefixPos - 1); + } + return NONE_PKG; +} + +const PackageInfo &PackageDB::getPackageInfo(core::NameRef mangledName) const { + auto it = packages.find(mangledName); + if (it == packages.end()) { + return NONE_PKG; + } + return *it->second; +} + +PackageDB PackageDB::deepCopy() const { + ENFORCE(frozen); + PackageDB result; + result.packages.reserve(this->packages.size()); + for (auto const &[nr, pkgInfo] : this->packages) { + result.packages[nr] = pkgInfo->deepCopy(); + } + result.packagesByPathPrefix = this->packagesByPathPrefix; + return result; +} + +UnfreezePackages PackageDB::unfreeze() { + return UnfreezePackages(*this); +} + +} // namespace sorbet::core::packages diff --git a/core/packages/PackageDB.h b/core/packages/PackageDB.h new file mode 100644 index 00000000000..e628b4afd35 --- /dev/null +++ b/core/packages/PackageDB.h @@ -0,0 +1,49 @@ +#ifndef SORBET_CORE_PACKAGES_PACKAGEDB_H +#define SORBET_CORE_PACKAGES_PACKAGEDB_H + +#include "common/common.h" +#include "core/Files.h" +#include "core/Names.h" +#include "core/packages/PackageInfo.h" + +namespace sorbet::core::packages { + +class PackageDB; + +class UnfreezePackages final { +public: + PackageDB &db; + + UnfreezePackages(PackageDB &); + ~UnfreezePackages(); +}; + +class PackageDB final { +public: + NameRef enterPackage(std::unique_ptr pkg); + NameRef lookupPackage(NameRef pkgMangledName) const; + + const PackageInfo &getPackageForFile(const core::GlobalState &gs, core::FileRef file) const; + const PackageInfo &getPackageInfo(core::NameRef mangledName) const; + + PackageDB deepCopy() const; + + UnfreezePackages unfreeze(); + + PackageDB() = default; + PackageDB(const PackageDB &) = delete; + PackageDB(PackageDB &&) = default; + PackageDB &operator=(const PackageDB &) = delete; + PackageDB &operator=(PackageDB &&) = default; + +private: + UnorderedMap> packages; + UnorderedMap packagesByPathPrefix; + bool frozen = true; + std::thread::id writerThread; + + friend class UnfreezePackages; +}; + +} // namespace sorbet::core::packages +#endif // SORBET_CORE_PACKAGES_PACKAGEDB_H diff --git a/core/packages/PackageInfo.cc b/core/packages/PackageInfo.cc new file mode 100644 index 00000000000..3c2a4d552b9 --- /dev/null +++ b/core/packages/PackageInfo.cc @@ -0,0 +1,15 @@ +#include "core/packages/PackageInfo.h" +#include "core/Loc.h" +#include "core/NameRef.h" + +using namespace std; + +namespace sorbet::core::packages { +bool PackageInfo::exists() const { + return mangledName().exists(); +} + +PackageInfo::~PackageInfo() { + // see https://eli.thegreenplace.net/2010/11/13/pure-virtual-destructors-in-c +} +} // namespace sorbet::core::packages diff --git a/core/packages/PackageInfo.h b/core/packages/PackageInfo.h new file mode 100644 index 00000000000..2206d566ca8 --- /dev/null +++ b/core/packages/PackageInfo.h @@ -0,0 +1,28 @@ +#ifndef SORBET_CORE_PACKAGES_PACKAGEINFO_H +#define SORBET_CORE_PACKAGES_PACKAGEINFO_H + +#include + +namespace sorbet::core { +class NameRef; +class Loc; +} // namespace sorbet::core + +namespace sorbet::core::packages { +class PackageInfo { +public: + virtual core::NameRef mangledName() const = 0; + virtual const std::vector &pathPrefixes() const = 0; + virtual std::unique_ptr deepCopy() const = 0; + virtual core::Loc definitionLoc() const = 0; + virtual bool exists() const final; + + virtual ~PackageInfo() = 0; + PackageInfo() = default; + PackageInfo(PackageInfo &) = delete; + explicit PackageInfo(const PackageInfo &) = default; + PackageInfo &operator=(PackageInfo &&) = delete; + PackageInfo &operator=(const PackageInfo &) = delete; +}; +} // namespace sorbet::core::packages +#endif diff --git a/main/lsp/LSPTypechecker.cc b/main/lsp/LSPTypechecker.cc index ae0fa9dc4dc..0decac51330 100644 --- a/main/lsp/LSPTypechecker.cc +++ b/main/lsp/LSPTypechecker.cc @@ -582,10 +582,12 @@ vector LSPTypechecker::getResolved(const vector ENFORCE(this_thread::get_id() == typecheckerThreadId, "Typechecker can only be used from the typechecker thread."); vector updatedIndexed; + bool addAllPackages = false; for (auto fref : frefs) { if (fref.data(*gs).sourceType == core::File::Type::Package) { // Will be added in second loop. ENFORCE(config->opts.stripePackages); + addAllPackages = true; continue; } @@ -595,8 +597,9 @@ vector LSPTypechecker::getResolved(const vector } } - if (config->opts.stripePackages) { - // We must include every package file to resolve these files properly. + if (addAllPackages) { + ENFORCE(config->opts.stripePackages); + // We must include every package file to rebuild the full PackageDB. for (u4 i = 1; i < gs->filesUsed(); i++) { core::FileRef fref(i); if (fref.data(*gs).sourceType == core::File::Type::Package) { diff --git a/packager/packager.cc b/packager/packager.cc index 80dc7fcbea1..18cb4f883a0 100644 --- a/packager/packager.cc +++ b/packager/packager.cc @@ -11,6 +11,7 @@ #include "common/sort.h" #include "core/Unfreeze.h" #include "core/errors/packager.h" +#include "core/packages/PackageInfo.h" #include using namespace std; @@ -97,7 +98,20 @@ struct Export { } }; -struct PackageInfo { +class PackageInfoImpl final : public sorbet::core::packages::PackageInfo { +public: + core::NameRef mangledName() const { + return name.mangledName; + } + + const std::vector &pathPrefixes() const { + return packagePathPrefixes; + } + + core::Loc definitionLoc() const { + return loc; + } + // The possible path prefixes associated with files in the package, including path separator at end. vector packagePathPrefixes; PackageName name; @@ -108,98 +122,25 @@ struct PackageInfo { // List of exported items that form the body of this package's public API. // These are copied into every package that imports this package. vector exports; -}; -/** - * Container class that facilitates thread-safe read-only access to packages. - */ -class PackageDB final { -private: - // The only thread that is allowed write access to this class. - const std::thread::id owner; - UnorderedMap> packageInfoByPathPrefix; - bool finalized = false; - UnorderedMap> packageInfoByMangledName; - -public: - PackageDB() : owner(this_thread::get_id()) {} - - void addPackage(core::Context ctx, shared_ptr pkg) { - ENFORCE(owner == this_thread::get_id()); - if (finalized) { - Exception::raise("Cannot add additional packages after finalizing PackageDB"); - } - if (pkg == nullptr) { - // There was an error creating a PackageInfo for this file, and getPackageInfo has already surfaced that - // error to the user. Nothing to do here. - return; - } - auto it = packageInfoByMangledName.find(pkg->name.mangledName); - if (it != packageInfoByMangledName.end()) { - if (auto e = ctx.beginError(pkg->loc.offsets(), core::errors::Packager::RedefinitionOfPackage)) { - auto pkgName = pkg->name.toString(ctx); - e.setHeader("Redefinition of package `{}`", pkgName); - e.addErrorLine(it->second->loc, "Package `{}` originally defined here", pkgName); - } - } else { - packageInfoByMangledName[pkg->name.mangledName] = pkg; - } - - for (const std::string &packagePathPrefix : pkg->packagePathPrefixes) { - packageInfoByPathPrefix[packagePathPrefix] = pkg; - } - } - - void finalizePackages() { - ENFORCE(owner == this_thread::get_id()); - finalized = true; + // PackageInfoImpl is the only implementation of PackageInfoImpl + const static PackageInfoImpl &from(const sorbet::core::packages::PackageInfo &pkg) { + ENFORCE(pkg.exists()); + return reinterpret_cast(pkg); // TODO is there a more idiomatic way to do this? } - /** - * Given a file of type PACKAGE, return its PackageInfo or nullptr if one does not exist. - */ - const PackageInfo *getPackageByFile(core::Context ctx, core::FileRef packageFile) const { - const std::string_view path = packageFile.data(ctx).path(); - const auto &it = packageInfoByPathPrefix.find(path.substr(0, path.find_last_of('/') + 1)); - if (it == packageInfoByPathPrefix.end()) { - return nullptr; - } - return it->second.get(); + static PackageInfoImpl &from(sorbet::core::packages::PackageInfo &pkg) { + ENFORCE(pkg.exists()); + return reinterpret_cast(pkg); // TODO is there a more idiomatic way to do this? } - /** - * Given the mangled name for a package (e.g., Foo::Bar's mangled name is Foo_Bar_Package), return that package's - * info or nullptr if it does not exist. - */ - const PackageInfo *getPackageByMangledName(core::NameRef name) const { - const auto &it = packageInfoByMangledName.find(name); - if (it == packageInfoByMangledName.end()) { - return nullptr; - } - return it->second.get(); + unique_ptr deepCopy() const { + return make_unique(*this); } - /** - * Given a context, return the active package or nullptr if one does not exist. - */ - const PackageInfo *getPackageForContext(core::Context ctx) const { - if (!finalized) { - Exception::raise("Cannot map files to packages until all packages are added and PackageDB is finalized"); - } - - std::string_view path = ctx.file.data(ctx).path(); - int curPrefixPos = path.find_last_of('/'); - while (curPrefixPos != std::string::npos) { - const auto it = packageInfoByPathPrefix.find(path.substr(0, curPrefixPos + 1)); - if (it != packageInfoByPathPrefix.end()) { - return it->second.get(); - } - - curPrefixPos = path.find_last_of('/', curPrefixPos - 1); - } - - return nullptr; - } + PackageInfoImpl() = default; + explicit PackageInfoImpl(const PackageInfoImpl &) = default; + PackageInfoImpl &operator=(const PackageInfoImpl &) = delete; }; void checkPackageName(core::Context ctx, ast::UnresolvedConstantLit *constLit) { @@ -328,15 +269,15 @@ bool sharesPrefix(const vector &a, const vector &b // Visitor that ensures for constants defined within a package that all have the package as a // prefix. class EnforcePackagePrefix final { - const PackageInfo *pkg; + const PackageInfoImpl &pkg; const bool isTestFile; vector nameParts; int rootConsts = 0; int skipPush = 0; public: - EnforcePackagePrefix(const PackageInfo *pkg, bool isTestFile) : pkg(pkg), isTestFile(isTestFile) { - ENFORCE(pkg != nullptr); + EnforcePackagePrefix(const PackageInfoImpl &pkg, bool isTestFile) : pkg(pkg), isTestFile(isTestFile) { + ENFORCE(pkg.exists()); } ast::ExpressionPtr preTransformClassDef(core::Context ctx, ast::ExpressionPtr tree) { @@ -436,15 +377,15 @@ class EnforcePackagePrefix final { const vector &requiredNamespace() const { if (isTestFile) { - return pkg->name.fullTestPkgName.parts; + return pkg.name.fullTestPkgName.parts; } else { - return pkg->name.fullName.parts; + return pkg.name.fullName.parts; } } }; struct PackageInfoFinder { - unique_ptr info = nullptr; + unique_ptr info = nullptr; vector exported; ast::ExpressionPtr postTransformSend(core::MutableContext ctx, ast::ExpressionPtr tree) { @@ -535,7 +476,7 @@ struct PackageInfoFinder { } } else if (info == nullptr) { auto nameTree = ast::cast_tree(classDef.name); - info = make_unique(); + info = make_unique(); checkPackageName(ctx, nameTree); info->name = getPackageName(ctx, nameTree); info->loc = core::Loc(ctx.file, classDef.loc); @@ -704,17 +645,10 @@ struct PackageInfoFinder { } }; -// TODO (aadi-stripe) we can avoid syscalls if we invent an efficient way of looking up -// directories in the source tree via GlobalState. Might be tied to https://github.com/sorbet/sorbet/issues/4509 -bool pathExists(const std::string &path) { - struct stat buffer; - return (stat(path.c_str(), &buffer) == 0); -} - // Sanity checks package files, mutates arguments to export / export_methods to point to item in namespace, // builds up the expression injected into packages that import the package, and codegens the module. -unique_ptr getPackageInfo(core::MutableContext ctx, ast::ParsedFile &package, - const vector &extraPackageFilesDirectoryPrefixes) { +unique_ptr getPackageInfo(core::MutableContext ctx, ast::ParsedFile &package, + const vector &extraPackageFilesDirectoryPrefixes) { ENFORCE(package.file.exists()); ENFORCE(package.file.data(ctx).sourceType == core::File::Type::Package); // Assumption: Root of AST is class. @@ -732,9 +666,7 @@ unique_ptr getPackageInfo(core::MutableContext ctx, ast::ParsedFile for (const string &prefix : extraPackageFilesDirectoryPrefixes) { string additionalDirPath = absl::StrCat(prefix, dirNameFromShortName, "/"); - if (pathExists(additionalDirPath)) { - finder.info->packagePathPrefixes.emplace_back(std::move(additionalDirPath)); - } + finder.info->packagePathPrefixes.emplace_back(std::move(additionalDirPath)); } } return move(finder.info); @@ -768,18 +700,18 @@ class ImportTree final { }; class ImportTreeBuilder final { - // PackageInfo package; // The package we are building an import tree for. + // PackageInfoImpl package; // The package we are building an import tree for. core::NameRef pkgMangledName; ImportTree root; public: - ImportTreeBuilder(const PackageInfo &package) : pkgMangledName(package.name.mangledName) {} + ImportTreeBuilder(const PackageInfoImpl &package) : pkgMangledName(package.name.mangledName) {} ImportTreeBuilder(const ImportTreeBuilder &) = delete; ImportTreeBuilder(ImportTreeBuilder &&) = default; ImportTreeBuilder &operator=(const ImportTreeBuilder &) = delete; ImportTreeBuilder &operator=(ImportTreeBuilder &&) = default; - void mergeImports(const PackageInfo &importedPackage, const Import &import) { + void mergeImports(const PackageInfoImpl &importedPackage, const Import &import) { for (const auto &exp : importedPackage.exports) { if (exp.type == ExportType::Public) { addImport(importedPackage, import.name.loc, exp.fqn, import.type); @@ -789,7 +721,7 @@ class ImportTreeBuilder final { // Make add imports for the test package for all normal code exported by its corresponding // "normal" package. - void mergeSelfExportsForTest(const PackageInfo &pkg) { + void mergeSelfExportsForTest(const PackageInfoImpl &pkg) { for (const auto &exp : pkg.exports) { const auto &parts = exp.parts(); ENFORCE(parts.size() > 0); @@ -808,7 +740,7 @@ class ImportTreeBuilder final { } private: - void addImport(const PackageInfo &importedPackage, core::LocOffsets loc, const FullyQualifiedName &exportFqn, + void addImport(const PackageInfoImpl &importedPackage, core::LocOffsets loc, const FullyQualifiedName &exportFqn, ImportType importType) { ImportTree *node = &root; for (auto nameRef : exportFqn.parts) { @@ -899,6 +831,47 @@ class ImportTreeBuilder final { } }; +ast::ParsedFile wrapFileInPackageModule(core::Context ctx, ast::ParsedFile file, core::NameRef packageMangledName, + const PackageInfoImpl &pkg, bool isTestFile) { + if (ast::isa_tree(file.tree)) { + // Nothing to wrap. This occurs when a file is marked typed: Ignore. + return file; + } + + auto &rootKlass = ast::cast_tree_nonnull(file.tree); + EnforcePackagePrefix enforcePrefix(pkg, isTestFile); + file.tree = ast::ShallowMap::apply(ctx, enforcePrefix, move(file.tree)); + + auto wrapperName = isTestFile ? core::Names::Constants::PackageTests() : core::Names::Constants::PackageRegistry(); + auto moduleWrapper = + ast::MK::Module(core::LocOffsets::none(), core::LocOffsets::none(), + name2Expr(packageMangledName, name2Expr(wrapperName)), {}, std::move(rootKlass.rhs)); + rootKlass.rhs.clear(); + rootKlass.rhs.emplace_back(move(moduleWrapper)); + return file; +} + +// We can't run packages without having all package ASTs. Assert that they are all present. +bool checkContainsAllPackages(const core::GlobalState &gs, const vector &files) { + UnorderedSet filePackages; + for (const auto &f : files) { + if (f.file.data(gs).sourceType == core::File::Type::Package) { + filePackages.insert(f.file); + } + } + + for (u4 i = 1; i < gs.filesUsed(); i++) { + core::FileRef fref(i); + if (fref.data(gs).sourceType == core::File::Type::Package && !filePackages.contains(fref)) { + return false; + } + } + + return true; +} + +} // namespace + // Add: // module ::Mangled_Name_Package // module A::B::C @@ -907,16 +880,18 @@ class ImportTreeBuilder final { // ... // end // ...to __package.rb files to set up the package namespace. -ast::ParsedFile rewritePackage(core::Context ctx, ast::ParsedFile file, const PackageDB &packageDB) { +ast::ParsedFile rewritePackage(core::Context ctx, ast::ParsedFile file) { ast::ClassDef::RHS_store importedPackages; ast::ClassDef::RHS_store testImportedPackages; - auto package = packageDB.getPackageByFile(ctx, file.file); - if (package == nullptr) { + const auto &packageDB = ctx.state.packageDB(); + auto &absPkg = packageDB.getPackageForFile(ctx, file.file); + if (!absPkg.exists()) { // We already produced an error on this package when producing its package info. // The correct course of action is to abort the transform. return file; } + auto &package = PackageInfoImpl::from(absPkg); // Sanity check: __package.rb files _must_ be typed: strict if (file.file.data(ctx).originalSigil < core::StrictLevel::Strict) { @@ -927,11 +902,11 @@ ast::ParsedFile rewritePackage(core::Context ctx, ast::ParsedFile file, const Pa { UnorderedMap importedNames; - ImportTreeBuilder treeBuilder(*package); - for (auto &import : package->importedPackageNames) { + ImportTreeBuilder treeBuilder(package); + for (auto &import : package.importedPackageNames) { auto &imported = import.name; - auto *importedPackage = packageDB.getPackageByMangledName(imported.mangledName); - if (importedPackage == nullptr) { + auto &importedPackage = packageDB.getPackageInfo(imported.mangledName); + if (!importedPackage.exists()) { if (auto e = ctx.beginError(imported.loc, core::errors::Packager::PackageNotFound)) { e.setHeader("Cannot find package `{}`", imported.toString(ctx)); } @@ -946,13 +921,13 @@ ast::ParsedFile rewritePackage(core::Context ctx, ast::ParsedFile file, const Pa } } else { importedNames[imported.mangledName] = imported.loc; - treeBuilder.mergeImports(*importedPackage, import); + treeBuilder.mergeImports(PackageInfoImpl::from(importedPackage), import); } } importedPackages = treeBuilder.makeModule(ctx, ImportType::Normal); - treeBuilder.mergeSelfExportsForTest(*package); + treeBuilder.mergeSelfExportsForTest(package); testImportedPackages = treeBuilder.makeModule(ctx, ImportType::Test); } @@ -969,47 +944,36 @@ ast::ParsedFile rewritePackage(core::Context ctx, ast::ParsedFile file, const Pa return file; } -ast::ParsedFile rewritePackagedFile(core::Context ctx, ast::ParsedFile file, core::NameRef packageMangledName, - const PackageInfo *pkg, bool isTestFile) { - if (ast::isa_tree(file.tree)) { - // Nothing to wrap. This occurs when a file is marked typed: Ignore. - return file; - } - - auto &rootKlass = ast::cast_tree_nonnull(file.tree); - EnforcePackagePrefix enforcePrefix(pkg, isTestFile); - file.tree = ast::ShallowMap::apply(ctx, enforcePrefix, move(file.tree)); - - auto wrapperName = isTestFile ? core::Names::Constants::PackageTests() : core::Names::Constants::PackageRegistry(); - auto moduleWrapper = - ast::MK::Module(core::LocOffsets::none(), core::LocOffsets::none(), - name2Expr(packageMangledName, name2Expr(wrapperName)), {}, std::move(rootKlass.rhs)); - rootKlass.rhs.clear(); - rootKlass.rhs.emplace_back(move(moduleWrapper)); - return file; -} - -// We can't run packages without having all package ASTs. Assert that they are all present. -bool checkContainsAllPackages(const core::GlobalState &gs, const vector &files) { - UnorderedSet filePackages; - for (const auto &f : files) { - if (f.file.data(gs).sourceType == core::File::Type::Package) { - filePackages.insert(f.file); +ast::ParsedFile rewritePackagedFile(core::GlobalState &gs, ast::ParsedFile parsedFile) { + auto &file = parsedFile.file.data(gs); + ENFORCE(file.sourceType != core::File::Type::Package); + core::Context ctx(gs, core::Symbols::root(), parsedFile.file); + auto &pkg = gs.packageDB().getPackageForFile(ctx, ctx.file); + if (pkg.exists()) { + auto &pkgImpl = PackageInfoImpl::from(pkg); + parsedFile = + wrapFileInPackageModule(ctx, move(parsedFile), pkgImpl.name.mangledName, pkgImpl, isTestFile(gs, file)); + } else { + // Don't transform, but raise an error on the first line. + if (auto e = ctx.beginError(core::LocOffsets{0, 0}, core::errors::Packager::UnpackagedFile)) { + e.setHeader("File `{}` does not belong to a package; add a `{}` file to one " + "of its parent directories", + ctx.file.data(gs).path(), "__package.rb"); } } + return parsedFile; +} - for (u4 i = 1; i < gs.filesUsed(); i++) { - core::FileRef fref(i); - if (fref.data(gs).sourceType == core::File::Type::Package && !filePackages.contains(fref)) { - return false; - } +// Re-write source files to be in packages. This is only called if no package definitions were +// changed. +vector rewritePackagedFilesFast(core::GlobalState &gs, vector files) { + Timer timeit(gs.tracer(), "packager.rewritePackagedFilesFast"); + for (auto i = 0; i < files.size(); i++) { + files[i] = rewritePackagedFile(gs, move(files[i])); } - - return true; + return files; } -} // namespace - vector Packager::run(core::GlobalState &gs, WorkerPool &workers, vector files, const vector &extraPackageFilesDirectoryPrefixes) { Timer timeit(gs.tracer(), "packager"); @@ -1017,19 +981,33 @@ vector Packager::run(core::GlobalState &gs, WorkerPool &workers fast_sort(files, [](const auto &a, const auto &b) -> bool { return a.file < b.file; }); // Step 1: Find packages and determine their imports/exports. - PackageDB packageDB; { Timer timeit(gs.tracer(), "packager.findPackages"); core::UnfreezeNameTable unfreeze(gs); + core::packages::UnfreezePackages packages = gs.unfreezePackages(); for (auto &file : files) { if (FileOps::getFileName(file.file.data(gs).path()) == PACKAGE_FILE_NAME) { file.file.data(gs).sourceType = core::File::Type::Package; core::MutableContext ctx(gs, core::Symbols::root(), file.file); - packageDB.addPackage(ctx, getPackageInfo(ctx, file, extraPackageFilesDirectoryPrefixes)); + auto pkg = getPackageInfo(ctx, file, extraPackageFilesDirectoryPrefixes); + if (pkg == nullptr) { + // There was an error creating a PackageInfoImpl for this file, and getPackageInfo has already + // surfaced that error to the user. Nothing to do here. + continue; + } + auto &prevPgk = gs.packageDB().getPackageInfo(pkg->mangledName()); + if (prevPgk.exists() && prevPgk.definitionLoc() != pkg->definitionLoc()) { + if (auto e = ctx.beginError(pkg->loc.offsets(), core::errors::Packager::RedefinitionOfPackage)) { + auto pkgName = pkg->name.toString(ctx); + e.setHeader("Redefinition of package `{}`", pkgName); + auto &prevPkg = gs.packageDB().getPackageInfo(pkg->mangledName()); + e.addErrorLine(prevPkg.definitionLoc(), "Package `{}` originally defined here", pkgName); + } + } else { + packages.db.enterPackage(move(pkg)); + } } } - // We're done adding packages. - packageDB.finalizePackages(); } { @@ -1038,7 +1016,7 @@ vector Packager::run(core::GlobalState &gs, WorkerPool &workers for (auto &file : files) { if (file.file.data(gs).sourceType == core::File::Type::Package) { core::Context ctx(gs, core::Symbols::root(), file.file); - file = rewritePackage(ctx, move(file), packageDB); + file = rewritePackage(ctx, move(file)); } } } @@ -1053,9 +1031,7 @@ vector Packager::run(core::GlobalState &gs, WorkerPool &workers fileq->push(move(file), 1); } - const PackageDB &constPkgDB = packageDB; - - workers.multiplexJob("rewritePackagedFiles", [&gs, constPkgDB, fileq, resultq]() { + workers.multiplexJob("rewritePackagedFiles", [&gs, fileq, resultq]() { Timer timeit(gs.tracer(), "packager.rewritePackagedFilesWorker"); vector results; u4 filesProcessed = 0; @@ -1065,18 +1041,7 @@ vector Packager::run(core::GlobalState &gs, WorkerPool &workers filesProcessed++; auto &file = job.file.data(gs); if (file.sourceType == core::File::Type::Normal) { - core::Context ctx(gs, core::Symbols::root(), job.file); - if (auto pkg = constPkgDB.getPackageForContext(ctx)) { - job = rewritePackagedFile(ctx, move(job), pkg->name.mangledName, pkg, isTestFile(gs, file)); - } else { - // Don't transform, but raise an error on the first line. - if (auto e = - ctx.beginError(core::LocOffsets{0, 0}, core::errors::Packager::UnpackagedFile)) { - e.setHeader("File `{}` does not belong to a package; add a `__package.rb` file to one " - "of its parent directories", - ctx.file.data(gs).path()); - } - } + job = rewritePackagedFile(gs, move(job)); } results.emplace_back(move(job)); } @@ -1107,13 +1072,20 @@ vector Packager::run(core::GlobalState &gs, WorkerPool &workers vector Packager::runIncremental(core::GlobalState &gs, vector files, const vector &extraPackageFilesDirectoryPrefixes) { - // Just run all packages w/ the changed files through Packager again. It should not define any new names. - // TODO(jvilk): This incremental pass reprocesses every package file in the project. It should instead only process - // the packages needed to understand file changes. - ENFORCE(checkContainsAllPackages(gs, files)); + // However, if only source files have changed the existing PackageDB can be used to re-process + // the changed files only. + // TODO(nroman-stripe) This could be further incrementalized to avoid processing all packages by + // building in an understanding of the dependencies between packages. auto namesUsed = gs.namesUsedTotal(); - auto emptyWorkers = WorkerPool::create(0, gs.tracer()); - files = Packager::run(gs, *emptyWorkers, move(files), extraPackageFilesDirectoryPrefixes); + bool packageDefChanged = absl::c_any_of( + files, [&gs](const auto &pf) -> bool { return pf.file.data(gs).sourceType == core::File::Type::Package; }); + if (packageDefChanged) { + ENFORCE(checkContainsAllPackages(gs, files)); + auto emptyWorkers = WorkerPool::create(0, gs.tracer()); + files = Packager::run(gs, *emptyWorkers, move(files), extraPackageFilesDirectoryPrefixes); + } else { + files = rewritePackagedFilesFast(gs, move(files)); + } ENFORCE(gs.namesUsedTotal() == namesUsed); return files; } diff --git a/packager/packager.h b/packager/packager.h index 5198f44d226..bcfa734d98b 100644 --- a/packager/packager.h +++ b/packager/packager.h @@ -1,6 +1,7 @@ #ifndef SORBET_REWRITER_PACKAGE_H #define SORBET_REWRITER_PACKAGE_H #include "ast/ast.h" +#include "core/packages/PackageInfo.h" namespace sorbet { class WorkerPool;