Desugar conditional sends

.bazelrc

@@ -71,6 +71,10 @@ build --cxxopt=-Wno-unqualified-std-cast-call --host_cxxopt=-Wno-unqualified-std
 build:dbg --copt=-O0
 build:dbg --compilation_mode=dbg
 build:dbg --config=debugsymbols
+# As of Bazel 7, cached files are not put into the `bazel-bin/` folder unless they're toplevel targets.
+# We want to force files to be downloaded because otherwise clangd won't find them (and possibly lldb, though I haven't checked)
+# TODO(jez) Do newer compile commands generators suffer from this problem? The one we use is archived
+build:dbg --remote_download_outputs=all
 
 build:rubydbg --copt=-DRUBY_DEBUG --copt=-DVM_CHECK_MODE=1 --copt=-DTRANSIENT_HEAP_CHECK_MODE --copt=-DRGENGC_CHECK_MODE --copt=-DENC_DEBUG
 

.buildkite/test-static-sanitized.sh

@@ -49,21 +49,30 @@ if [ "$err" -ne 0 ]; then
   echo 'Run this command to run failing tests locally:' >> "$failing_tests"
   echo >> "$failing_tests"
   echo '```bash' >> "$failing_tests"
-  echo "./bazel test \\" >> "$failing_tests"
 
   # Take the lines that start with target labels.
   # Lines look like "//foo  FAILED in 10s"
-  { ./bazel test --test_summary=terse "${test_args[@]}" || true ; } | \
-    grep '^//' | \
-    sed -e 's/ .*/ \\/' | \
-    sed -e 's/^/  /' >> "$failing_tests"
+  failing_targets=$(
+    { ./bazel test --test_summary=terse "${test_args[@]}" || true ; } | \
+      grep '^//' | \
+      sed -e 's/ .*//'
+  )
+
+  num_failing=$(echo "$failing_targets" | wc -l | tr -d ' ')
 
-  # Put this last as an easy way to not have a `\` on the last line.
-  #
   # Use --config=dbg instead of sanitized because it's more likely that they've
   # already built this config locally, and most test failures reproduce outside
   # of sanitized mode anyways.
-  echo '  --config=dbg --test_output=errors' >> "$failing_tests"
+  if [ "$num_failing" -eq 1 ]; then
+    # Single test - put everything on one line
+    echo "./bazel test --config=dbg --test_output=errors $failing_targets" >> "$failing_tests"
+  else
+    # Multiple tests - use multi-line format
+    echo "./bazel test --config=dbg --test_output=errors \\" >> "$failing_tests"
+    # Add backslash to all but the last line, indent all lines
+    echo "$failing_targets" | sed -e '$!s/$/ \\/' -e 's/^/  /' >> "$failing_tests"
+  fi
+
   echo '```' >> "$failing_tests"
 
   buildkite-agent annotate --context "test-static-sanitized.sh" --style error --append < "$failing_tests"

.vscode/settings.json

@@ -10,13 +10,7 @@
   // Disable feature where Clangd auto-imports headers for missing references.
   // It inserts relative paths that we don't want.
   "clangd.arguments": ["--header-insertion=never"],
-  // TODO(jez) Something changed and now clangd doesn't work when we invoke the
-  // one out of the bazel sandbox. The one installed to /usr/bin from XCode mostly
-  // works, modulo some extra warnings, so let's just use that until we figure out
-  // a better way forward.
-  // TODO(jez) Revisit when we next upgrade llvm_toolchain
-  //"clangd.path": "bazel-sorbet/external/llvm_toolchain_15_0_7/bin/clangd",
-  "clangd.path": "clangd",
+  "clangd.path": "bazel-sorbet/external/llvm_toolchain_15_0_7/bin/clangd",
   "files.associations": {
     "*.rbi": "ruby",
     "*.rbupdated": "ruby",

ast/desugar/Desugar.cc

@@ -203,6 +203,9 @@ ExpressionPtr desugarBlock(DesugarContext dctx, parser::Block *block) {
     } else {
         // This must have been a csend; That will have been desugared
         // into an insseq with an If in the expression.
+        //
+        // Bug: This could also be an `If` created by desugaring a `block_given?` call.
+        // https://github.com/sorbet/sorbet/issues/9860
         res = move(recv);
         auto is = cast_tree<InsSeq>(res);
         if (!is) {

ast/desugar/PrismDesugar.cc

@@ -22,62 +22,15 @@ namespace sorbet::ast::prismDesugar {
 
 using namespace std;
 
-namespace {
-
-// Translate a tree to an expression. NOTE: this should only be called from `node2TreeImpl`.
-ExpressionPtr node2TreeImplBody(parser::Node *what) {
-    ENFORCE(what != nullptr);
-    ENFORCE(what->hasDesugaredExpr(), "Node has no desugared expression");
-
-    auto expr = what->takeDesugaredExpr();
-    ENFORCE(expr != nullptr, "Node has null desugared expr");
-
-    return expr;
-}
-
-// Translate trees by calling `node2TreeBody`, and manually reset the unique_ptr argument when it's done.
-ExpressionPtr node2TreeImpl(unique_ptr<parser::Node> &what) {
-    auto res = node2TreeImplBody(what.get());
-    what.reset();
-    return res;
-}
-
-ExpressionPtr liftTopLevel(core::LocOffsets loc, ExpressionPtr what) {
-    ClassDef::RHS_store rhs;
-    ClassDef::ANCESTORS_store ancestors;
-    ancestors.emplace_back(MK::Constant(loc, core::Symbols::todo()));
-    auto insSeq = cast_tree<InsSeq>(what);
-    if (insSeq) {
-        rhs.reserve(insSeq->stats.size() + 1);
-        for (auto &stat : insSeq->stats) {
-            rhs.emplace_back(move(stat));
-        }
-        rhs.emplace_back(move(insSeq->expr));
-    } else {
-        rhs.emplace_back(move(what));
-    }
-    return make_expression<ClassDef>(loc, loc, core::Symbols::root(), MK::EmptyTree(), move(ancestors), move(rhs),
-                                     ClassDef::Kind::Class);
-}
-} // namespace
-
 ExpressionPtr node2Tree(core::MutableContext ctx, unique_ptr<parser::Node> what, bool preserveConcreteSyntax) {
     try {
         // Callers should not pass null - when Prism falls back, use legacy desugar instead
         ENFORCE(what != nullptr, "node2Tree called with null tree");
+        ENFORCE(what->hasDesugaredExpr(), "Node has no desugared expression");
+
+        auto result = what->takeDesugaredExpr();
+        ENFORCE(result != nullptr, "Node has null desugared expr");
 
-        auto liftedClassDefLoc = what->loc;
-        auto result = node2TreeImpl(what);
-        if (result.loc().exists()) {
-            // If the desugared expression has a different loc, we want to use that. This can happen
-            // because (:block (:send)) desugars to (:send (:block)), but the (:block) node just has
-            // the loc of the `do ... end`, while the (:send) has the whole loc
-            //
-            // But if we desugared to EmptyTree (either intentionally or because there was an
-            // unsupported node type), we want to use the loc of the original node.
-            liftedClassDefLoc = result.loc();
-        }
-        result = liftTopLevel(liftedClassDefLoc, move(result));
         auto verifiedResult = Verifier::run(ctx, move(result));
         return verifiedResult;
     } catch (SorbetException &) {

common/exception/Exception.h

@@ -75,5 +75,12 @@ template <typename... TArgs> [[noreturn]] bool Exception::raise(fmt::format_stri
     stopInDebugger();
     throw SorbetException(message);
 }
+
+// Indicates that a particular code path should never be reached, with an explanation of why.
+// Throws a `sorbet::SorbetException` when triggered to help with debugging.
+template <typename... TArgs>
+[[noreturn]] void unreachable(fmt::format_string<TArgs...> reasonFormatStr, TArgs &&...args) {
+    Exception::raise(reasonFormatStr, std::forward<TArgs>(args)...);
+}
 } // namespace sorbet
 #endif // SORBET_ERRO_H

core/GlobalState.cc

@@ -995,8 +995,7 @@ void GlobalState::computeLinearization() {
     Timer timer(this->tracer(), "resolver.compute_linearization");
 
     // TODO: this does not support `prepend`
-    for (int i = 1; i < this->classAndModulesUsed(); ++i) {
-        const auto &ref = core::ClassOrModuleRef(*this, i);
+    for (auto ref : this->newClassOrModules()) {
         computeClassLinearization(*this, ref);
     }
 }
@@ -2070,6 +2069,7 @@ unique_ptr<GlobalState> GlobalState::deepCopyGlobalState(bool keepId) const {
         Timer timeit2(tracer(), "GlobalState::deepCopyOut");
         result->creation = timeit2.getFlowEdge();
     }
+    result->symbolOffsets = this->symbolOffsets;
     return result;
 }
 
@@ -2078,7 +2078,8 @@ unique_ptr<GlobalState> GlobalState::copyForIndexThread(
     const vector<string> &extraPackageFilesDirectorySlashDeprecatedPrefixes,
     const vector<string> &extraPackageFilesDirectorySlashPrefixes,
     const vector<string> &packageSkipRBIExportEnforcementDirs, const vector<string> &allowRelaxedPackagerChecksFor,
-    const vector<string> &packagerLayers, string errorHint, bool genPackages) const {
+    const vector<string> &updateVisibilityFor, const vector<string> &packagerLayers, string errorHint,
+    bool genPackages) const {
     ENFORCE(fileTableFrozen);
     auto result = make_unique<GlobalState>(this->errorQueue, this->epochManager);
 
@@ -2096,10 +2097,10 @@ unique_ptr<GlobalState> GlobalState::copyForIndexThread(
     if (packagerEnabled) {
         core::UnfreezeNameTable unfreezeToEnterPackagerOptionsGS(*result);
         core::packages::UnfreezePackages unfreezeToEnterPackagerOptionsPackageDB = result->unfreezePackages();
-        result->setPackagerOptions(extraPackageFilesDirectoryUnderscorePrefixes,
-                                   extraPackageFilesDirectorySlashDeprecatedPrefixes,
-                                   extraPackageFilesDirectorySlashPrefixes, packageSkipRBIExportEnforcementDirs,
-                                   allowRelaxedPackagerChecksFor, packagerLayers, errorHint, genPackages);
+        result->setPackagerOptions(
+            extraPackageFilesDirectoryUnderscorePrefixes, extraPackageFilesDirectorySlashDeprecatedPrefixes,
+            extraPackageFilesDirectorySlashPrefixes, packageSkipRBIExportEnforcementDirs, allowRelaxedPackagerChecksFor,
+            updateVisibilityFor, packagerLayers, errorHint, genPackages);
     }
 
     return result;
@@ -2110,7 +2111,8 @@ unique_ptr<GlobalState> GlobalState::copyForLSPTypechecker(
     const vector<string> &extraPackageFilesDirectorySlashDeprecatedPrefixes,
     const vector<string> &extraPackageFilesDirectorySlashPrefixes,
     const vector<string> &packageSkipRBIExportEnforcementDirs, const vector<string> &allowRelaxedPackagerChecksFor,
-    const vector<string> &packagerLayers, string errorHint, bool genPackages) const {
+    const vector<string> &updateVisibilityFor, const vector<string> &packagerLayers, string errorHint,
+    bool genPackages) const {
     auto result = make_unique<GlobalState>(this->errorQueue, this->epochManager);
 
     result->initEmpty();
@@ -2124,10 +2126,10 @@ unique_ptr<GlobalState> GlobalState::copyForLSPTypechecker(
     if (packagerEnabled) {
         core::UnfreezeNameTable unfreezeToEnterPackagerOptionsGS(*result);
         core::packages::UnfreezePackages unfreezeToEnterPackagerOptionsPackageDB = result->unfreezePackages();
-        result->setPackagerOptions(extraPackageFilesDirectoryUnderscorePrefixes,
-                                   extraPackageFilesDirectorySlashDeprecatedPrefixes,
-                                   extraPackageFilesDirectorySlashPrefixes, packageSkipRBIExportEnforcementDirs,
-                                   allowRelaxedPackagerChecksFor, packagerLayers, errorHint, genPackages);
+        result->setPackagerOptions(
+            extraPackageFilesDirectoryUnderscorePrefixes, extraPackageFilesDirectorySlashDeprecatedPrefixes,
+            extraPackageFilesDirectorySlashPrefixes, packageSkipRBIExportEnforcementDirs, allowRelaxedPackagerChecksFor,
+            updateVisibilityFor, packagerLayers, errorHint, genPackages);
     }
 
     return result;
@@ -2137,7 +2139,8 @@ GlobalState::copyForSlowPath(const vector<string> &extraPackageFilesDirectoryUnd
                              const vector<string> &extraPackageFilesDirectorySlashDeprecatedPrefixes,
                              const vector<string> &extraPackageFilesDirectorySlashPrefixes,
                              const vector<string> &packageSkipRBIExportEnforcementDirs,
-                             const vector<string> &allowRelaxedPackagerChecksFor, const vector<string> &packagerLayers,
+                             const vector<string> &allowRelaxedPackagerChecksFor,
+                             const vector<string> &updateVisibilityFor, const vector<string> &packagerLayers,
                              string errorHint, bool genPackages) const {
     auto result = make_unique<GlobalState>(this->errorQueue, this->epochManager);
 
@@ -2168,10 +2171,10 @@ GlobalState::copyForSlowPath(const vector<string> &extraPackageFilesDirectoryUnd
     if (packageDB().enabled()) {
         core::UnfreezeNameTable unfreezeToEnterPackagerOptionsGS(*result);
         core::packages::UnfreezePackages unfreezeToEnterPackagerOptionsPackageDB = result->unfreezePackages();
-        result->setPackagerOptions(extraPackageFilesDirectoryUnderscorePrefixes,
-                                   extraPackageFilesDirectorySlashDeprecatedPrefixes,
-                                   extraPackageFilesDirectorySlashPrefixes, packageSkipRBIExportEnforcementDirs,
-                                   allowRelaxedPackagerChecksFor, packagerLayers, errorHint, genPackages);
+        result->setPackagerOptions(
+            extraPackageFilesDirectoryUnderscorePrefixes, extraPackageFilesDirectorySlashDeprecatedPrefixes,
+            extraPackageFilesDirectorySlashPrefixes, packageSkipRBIExportEnforcementDirs, allowRelaxedPackagerChecksFor,
+            updateVisibilityFor, packagerLayers, errorHint, genPackages);
     }
 
     return result;
@@ -2322,7 +2325,8 @@ void GlobalState::setPackagerOptions(const vector<string> &extraPackageFilesDire
                                      const vector<string> &extraPackageFilesDirectorySlashPrefixes,
                                      const vector<string> &packageSkipRBIExportEnforcementDirs,
                                      const vector<string> &allowRelaxedPackagerChecksFor,
-                                     const vector<string> &packagerLayers, string errorHint, bool genPackages) {
+                                     const vector<string> &updateVisibilityFor, const vector<string> &packagerLayers,
+                                     string errorHint, bool genPackages) {
     ENFORCE_NO_TIMER(!packageDB_.frozen);
 
     packageDB_.enabled_ = true;
@@ -2332,6 +2336,7 @@ void GlobalState::setPackagerOptions(const vector<string> &extraPackageFilesDire
     packageDB_.extraPackageFilesDirectorySlashPrefixes_ = extraPackageFilesDirectorySlashPrefixes;
     packageDB_.skipRBIExportEnforcementDirs_ = packageSkipRBIExportEnforcementDirs;
     packageDB_.allowRelaxedPackagerChecksFor_ = allowRelaxedPackagerChecksFor;
+    packageDB_.updateVisibilityFor_ = updateVisibilityFor;
     absl::c_transform(packagerLayers, std::back_inserter(packageDB_.layers_),
                       [this](const auto &layer) { return enterNameUTF8(layer); });
     packageDB_.errorHint_ = errorHint;
@@ -2514,4 +2519,28 @@ spdlog::logger &GlobalState::tracer() const {
     return errorQueue->tracer;
 }
 
+SymbolTableOffsets::SymbolTableOffsets(const core::GlobalState &gs)
+    : classAndModulesOffset{gs.classAndModulesUsed()}, methodsOffset{gs.methodsUsed()}, fieldsOffset{gs.fieldsUsed()},
+      typeMembersOffset{gs.typeMembersUsed()}, typeParametersOffset{gs.typeParametersUsed()} {}
+
+SymbolRange<ClassOrModuleRef> SymbolTableOffsets::classOrModuleRefs(const GlobalState &gs) const {
+    return SymbolRange<ClassOrModuleRef>(this->classAndModulesOffset, gs.classAndModulesUsed());
+}
+
+SymbolRange<MethodRef> SymbolTableOffsets::methodRefs(const GlobalState &gs) const {
+    return SymbolRange<MethodRef>(this->methodsOffset, gs.methodsUsed());
+}
+
+SymbolRange<FieldRef> SymbolTableOffsets::fieldRefs(const GlobalState &gs) const {
+    return SymbolRange<FieldRef>(this->fieldsOffset, gs.fieldsUsed());
+}
+
+SymbolRange<TypeMemberRef> SymbolTableOffsets::typeMemberRefs(const GlobalState &gs) const {
+    return SymbolRange<TypeMemberRef>(this->typeMembersOffset, gs.typeMembersUsed());
+}
+
+SymbolRange<TypeParameterRef> SymbolTableOffsets::typeParameterRefs(const GlobalState &gs) const {
+    return SymbolRange<TypeParameterRef>(this->typeParametersOffset, gs.typeParametersUsed());
+}
+
 } // namespace sorbet::core