Crash after editing file: Module loc out of bounds

jvilk-stripe commented 3 years ago

Input

# typed: true

# This is a very long comment. A long comment so that the ::GRPC::GenericService definition at the end of the file
# is at a location that is not within the range of the rbupdate file.

Bundler::CRPC # error: Unable to resolve constant

module ::DRPC::SomeSubmodule; end

Remove the comment to see the crash. Note: It does not occur in web builds.

Observed output

In debug build:

[error] Exception::raise(): core/Loc.cc:68 enforced condition off <= file.source().size() has failed: file offset out of bounds in file: test/testdata/lsp/fast_path/crash.rb @ 259 <= 129

[error] Backtrace:
  #3 0x1783177 sorbet::core::Loc::offset2Pos()
  #4 0x17834d8 sorbet::core::Loc::position()
  #5 0x1784773 sorbet::core::Loc::filePosToString()
  #6 0x171efdc sorbet::core::ErrorLine::toString()
  #7 0x171f5cf sorbet::core::ErrorSection::toString()
  #8 0x171fa70 sorbet::core::Error::toString()
  #9 0x1731eee sorbet::core::ErrorQueue::pushError()
  #10 0x1767d5e sorbet::core::GlobalState::_error()
  #11 0x1720946 sorbet::core::ErrorBuilder::~ErrorBuilder()
  #12 0x13de913 sorbet::resolver::(anonymous namespace)::ResolveConstantsWalk::constantResolutionFailed()
  #13 0x13d62a3 sorbet::resolver::(anonymous namespace)::ResolveConstantsWalk::resolveConstants()
  #14 0x13d9f25 sorbet::resolver::Resolver::runIncremental()
  #15 0xf771b3 sorbet::realmain::pipeline::incrementalResolve()
  #16 0xd2fc78 sorbet::realmain::lsp::LSPTypechecker::runFastPath()
  #17 0xd2d299 sorbet::realmain::lsp::LSPTypechecker::typecheck()
  #18 0xd32eaf sorbet::realmain::lsp::LSPTypecheckerDelegate::typecheckOnFastPath()
  #19 0xe3e36c sorbet::realmain::lsp::SorbetWorkspaceEditTask::run()
  #20 0xe495db sorbet::realmain::lsp::(anonymous namespace)::TypecheckerTask::run()
  #21 0xe4768e sorbet::realmain::lsp::LSPTypecheckerCoordinator::asyncRunInternal()
  #22 0xe4775e sorbet::realmain::lsp::LSPTypecheckerCoordinator::syncRun()
  #23 0xd8772c sorbet::realmain::lsp::LSPLoop::runTask()
  #24 0xd87277 sorbet::realmain::lsp::LSPLoop::processRequests()
  #25 0xcfdbd3 sorbet::realmain::lsp::SingleThreadedLSPWrapper::getLSPResponsesFor()
  #26 0xc2d5be sorbet::test::getLSPResponsesFor()
  #27 0xb3eb38 sorbet::test::_DOCTEST_ANON_FUNC_52()
  #28 0x19bd8a9 doctest::Context::run()
  #29 0xb4016a main
  #30 0x7fede4f97840 __libc_start_main
  #31 0xb37029 _start

Expected behavior

No crash!

The issue occurs in a Sorbet error message that suggests an alternative constant since Bundler::CRPC does not exist. It suggests ::DRPC, which is defined at the end of the file.

When you remove the comment, Sorbet fails to update the location information for ::DRPC's definition, and tries to print a code location that does not exist.

This problem does not occur if we simply define module ::DRPC instead of module ::DRPC::SomeSubmodule, which leads me to believe that there is a bug in incremental namer when entering a submodule on a module that has not yet been defined elsewhere.

jvilk-stripe commented 3 years ago

OK, here's what I believe is happening:

We create the DRPC symbol implicitly because it does not exist when Sorbet evaluates DRPC::SomeSubmodule. Thus, DRPC ends up having a loc in this file.
Normally, when Sorbet encounters module Foo::Bar and Foo exists, it does not add a loc to Foo.
On the fast path, DRPC exists, so Sorbet does not update its loc when it encounters DRPC::SomeSubmodule.

I'll have to think on if a tractable solution is possible here.

jvilk-stripe commented 3 years ago

I tried a different approach that assigns a none Loc to these classes, but that ends up propagating to the singleton classes we create and we don't update their loc when we process the actual definition (ugh).

Maybe I can force singletonClass to update the singleton class loc or something.

jez commented 2 years ago

This looks like the same issue as #1587 but I can't reproduce using this example either. Maybe we fixed it at one point.

sorbet / sorbet