Skip to content

fix: Fix occurrences for field inheritance #108

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 1 commit into from
Sep 19, 2022
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: 2 additions & 0 deletions core/GlobalState.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1962,6 +1962,7 @@ unique_ptr<GlobalState> GlobalState::deepCopy(bool keepId) const {
result->sleepInSlowPathSeconds = this->sleepInSlowPathSeconds;
result->requiresAncestorEnabled = this->requiresAncestorEnabled;
result->lspExperimentalFastPathEnabled = this->lspExperimentalFastPathEnabled;
result->isSCIPRuby = this->isSCIPRuby;

if (keepId) {
result->globalStateId = this->globalStateId;
Expand Down Expand Up @@ -2055,6 +2056,7 @@ unique_ptr<GlobalState> GlobalState::copyForIndex() const {
result->runningUnderAutogen = this->runningUnderAutogen;
result->censorForSnapshotTests = this->censorForSnapshotTests;
result->lspExperimentalFastPathEnabled = this->lspExperimentalFastPathEnabled;
result->isSCIPRuby = this->isSCIPRuby;
result->sleepInSlowPathSeconds = this->sleepInSlowPathSeconds;
result->requiresAncestorEnabled = this->requiresAncestorEnabled;
result->kvstoreUuid = this->kvstoreUuid;
Expand Down
7 changes: 7 additions & 0 deletions core/GlobalState.h
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,13 @@ class GlobalState final {
// If 'true', enable the experimental, symbol-deletion-based fast path mode
bool lspExperimentalFastPathEnabled = false;

// If 'true', we're running in scip-ruby mode.
bool isSCIPRuby = true;

// --- begin scip-ruby specific state
UnorderedMap<core::ClassOrModuleRef, UnorderedSet<core::NameRef>> unresolvedFields;
// --- end scip-ruby specific state

// When present, this indicates that single-package rbi generation is being performed, and contains metadata about
// the packages that are imported by the one whose interface is being generated.
std::optional<packages::ImportInfo> singlePackageImports;
Expand Down
90 changes: 90 additions & 0 deletions resolver/resolver.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3924,6 +3924,93 @@ class ResolveSignaturesWalk {
}
};

/// Helper type to traverse ASTs and aggregate information about unresolved fields.
///
/// For perf, it would make sense to fuse this along with some other map-reduce
/// operation over files, but I've kept it separate for now for ease of maintenance.
class CollectUnresolvedFieldsWalk final {
UnorderedMap<core::ClassOrModuleRef, UnorderedSet<core::NameRef>> unresolvedFields;

public:
void postTransformUnresolvedIdent(core::Context ctx, ast::ExpressionPtr &tree) {
auto &unresolvedIdent = ast::cast_tree_nonnull<ast::UnresolvedIdent>(tree);
using Kind = ast::UnresolvedIdent::Kind;
core::ClassOrModuleRef klass;
switch (unresolvedIdent.kind) {
case Kind::Global:
case Kind::Local:
return;
case Kind::Class: {
auto enclosingClass = ctx.owner.enclosingClass(ctx);
klass = enclosingClass.data(ctx)->isSingletonClass(ctx)
? enclosingClass // in <static-init>
: enclosingClass.data(ctx)->lookupSingletonClass(ctx); // in static methods
break;
}
case Kind::Instance: {
klass = ctx.owner.enclosingClass(ctx);
}
}
this->unresolvedFields[klass].insert(unresolvedIdent.name);
}

struct CollectWalkResult {
UnorderedMap<core::ClassOrModuleRef, UnorderedSet<core::NameRef>> unresolvedFields;
vector<ast::ParsedFile> trees;
};

static vector<ast::ParsedFile> collect(core::GlobalState &gs, vector<ast::ParsedFile> trees, WorkerPool &workers) {
Timer timeit(gs.tracer(), "resolver.collect_unresolved_fields");
const core::GlobalState &igs = gs;
auto resultq = make_shared<BlockingBoundedQueue<CollectWalkResult>>(trees.size());
auto fileq = make_shared<ConcurrentBoundedQueue<ast::ParsedFile>>(trees.size());
for (auto &tree : trees) {
fileq->push(move(tree), 1);
}
trees.clear();

workers.multiplexJob("collectUnresolvedFieldsWalk", [&igs, fileq, resultq]() {
Timer timeit(igs.tracer(), "CollectUnresolvedFieldsWorker");
CollectUnresolvedFieldsWalk collect;
CollectWalkResult walkResult;
vector<ast::ParsedFile> collectedTrees;
ast::ParsedFile job;
for (auto result = fileq->try_pop(job); !result.done(); result = fileq->try_pop(job)) {
if (!result.gotItem()) {
continue;
}
core::Context ictx(igs, core::Symbols::root(), job.file);
ast::TreeWalk::apply(ictx, collect, job.tree);
collectedTrees.emplace_back(move(job));
}
if (!collectedTrees.empty()) {
walkResult.trees = move(collectedTrees);
walkResult.unresolvedFields = std::move(collect.unresolvedFields);
resultq->push(move(walkResult), walkResult.trees.size());
}
});

{
CollectWalkResult threadResult;
for (auto result = resultq->wait_pop_timed(threadResult, WorkerPool::BLOCK_INTERVAL(), gs.tracer());
!result.done();
result = resultq->wait_pop_timed(threadResult, WorkerPool::BLOCK_INTERVAL(), gs.tracer())) {
if (!result.gotItem()) {
continue;
}
trees.insert(trees.end(), make_move_iterator(threadResult.trees.begin()),
make_move_iterator(threadResult.trees.end()));
gs.unresolvedFields.reserve(gs.unresolvedFields.size() + threadResult.unresolvedFields.size());
gs.unresolvedFields.insert(make_move_iterator(threadResult.unresolvedFields.begin()),
make_move_iterator(threadResult.unresolvedFields.end()));
}
}

fast_sort(trees, [](const auto &lhs, const auto &rhs) -> bool { return lhs.file < rhs.file; });
return trees;
}
};

class ResolveSanityCheckWalk {
public:
void postTransformClassDef(core::Context ctx, ast::ExpressionPtr &tree) {
Expand Down Expand Up @@ -4102,6 +4189,9 @@ ast::ParsedFilesOrCancelled Resolver::run(core::GlobalState &gs, vector<ast::Par

auto result = resolveSigs(gs, std::move(rtmafResult.trees), workers);
ResolveTypeMembersAndFieldsWalk::resolvePendingCastItems(gs, rtmafResult.todoResolveCastItems);
if (gs.isSCIPRuby) {
result = CollectUnresolvedFieldsWalk::collect(gs, std::move(result), workers);
}
sanityCheck(gs, result);

return result;
Expand Down
120 changes: 114 additions & 6 deletions scip_indexer/SCIPIndexer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include "cfg/CFG.h"
#include "common/common.h"
#include "common/sort.h"
#include "core/Error.h"
#include "core/ErrorQueue.h"
#include "core/Loc.h"
#include "core/SymbolRef.h"
Expand Down Expand Up @@ -57,6 +58,29 @@ static uint32_t fnv1a_32(const string &s) {

namespace sorbet::scip_indexer {

constexpr sorbet::core::ErrorClass SCIPRubyDebug{400, sorbet::core::StrictLevel::False};

void _log_debug(const sorbet::core::GlobalState &gs, sorbet::core::Loc loc, std::string s) {
if (auto e = gs.beginError(loc, SCIPRubyDebug)) {
auto lines = absl::StrSplit(s, '\n');
for (auto line = lines.begin(); line != lines.end(); line++) {
auto text = string(line->begin(), line->length());
if (line == lines.begin()) {
e.setHeader("[scip-ruby] {}", text);
} else {
e.addErrorNote("{}", text);
}
}
}
}

#ifndef NDEBUG
#define LOG_DEBUG(__gs, __loc, __s) _log_debug(__gs, __loc, __s)
#else
#define LOG_DEBUG(__gs, __s) \
{}
#endif

// TODO(varun): This is an inline workaround for https://github.com/sorbet/sorbet/issues/5925
// I've not changed the main definition because I didn't bother to rerun the tests with the change.
static bool isTemporary(const core::GlobalState &gs, const core::LocalVariable &var) {
Expand Down Expand Up @@ -778,6 +802,63 @@ string format_ancestry(const core::GlobalState &gs, core::SymbolRef sym) {
return out.str();
}

static absl::variant</*owner*/ core::ClassOrModuleRef, core::SymbolRef>
findUnresolvedFieldTransitive(const core::GlobalState &gs, core::Loc loc, core::ClassOrModuleRef start,
core::NameRef field) {
auto fieldText = field.shortName(gs);
auto isInstanceVar = fieldText.size() >= 2 && fieldText[0] == '@' && fieldText[1] != '@';
auto isClassInstanceVar = isInstanceVar && start.data(gs)->isSingletonClass(gs);
// Class instance variables are not inherited, unlike ordinary instance
// variables or class variables.
if (isClassInstanceVar) {
return start;
}
auto isClassVar = fieldText.size() >= 2 && fieldText[0] == '@' && fieldText[1] == '@';
if (isClassVar && !start.data(gs)->isSingletonClass(gs)) {
// Triggered when undeclared class variables are accessed from instance methods.
start = start.data(gs)->lookupSingletonClass(gs);
}

// TODO(varun): Should we add a cache here? It seems wasteful to redo
// work for every occurrence.
if (gs.unresolvedFields.find(start) == gs.unresolvedFields.end() ||
!gs.unresolvedFields.find(start)->second.contains(field)) {
// Triggered by code patterns like:
// # top-level
// def MyClass.method
// # blah
// end
// which is not supported by Sorbet.
LOG_DEBUG(gs, loc,
fmt::format("couldn't find field {} in class {};\n"
"are you using a code pattern like def MyClass.method which is unsupported by Sorbet?",
field.exists() ? field.toString(gs) : "<non-existent>",
start.exists() ? start.showFullName(gs) : "<non-existent>"));
// As a best-effort guess, assume that the definition is
// in this class but we somehow missed it.
return start;
}

auto best = start;
auto cur = start;
while (cur.exists()) {
auto klass = cur.data(gs);
auto sym = klass->findMember(gs, field);
if (sym.exists()) {
return sym;
}
auto it = gs.unresolvedFields.find(cur);
if (it != gs.unresolvedFields.end() && it->second.contains(field)) {
best = cur;
}
if (cur == klass->superClass()) { // FIXME(varun): Handle mix-ins
break;
}
cur = klass->superClass();
}
return best;
}

// Loosely inspired by AliasesAndKeywords in IREmitterContext.cc
class AliasMap final {
public:
Expand Down Expand Up @@ -816,9 +897,27 @@ class AliasMap final {
}
if (sym == core::Symbols::Magic_undeclaredFieldStub()) {
ENFORCE(!bind.loc.empty());
this->map.insert( // no trim(...) because undeclared fields shouldn't have ::
{bind.bind.variable,
{NamedSymbolRef::undeclaredField(klass, instr->name, bind.bind.type), bind.loc, false}});
ENFORCE(klass.isClassOrModule());
auto result = findUnresolvedFieldTransitive(ctx, ctx.locAt(bind.loc), klass.asClassOrModuleRef(),
instr->name);
if (absl::holds_alternative<core::ClassOrModuleRef>(result)) {
auto klass = absl::get<core::ClassOrModuleRef>(result);
if (klass.exists()) {
this->map.insert( // no trim(...) because undeclared fields shouldn't have ::
{bind.bind.variable,
{NamedSymbolRef::undeclaredField(klass, instr->name, bind.bind.type), bind.loc,
false}});
}
} else if (absl::holds_alternative<core::SymbolRef>(result)) {
auto fieldSym = absl::get<core::SymbolRef>(result);
if (fieldSym.exists()) {
this->map.insert(
{bind.bind.variable,
{NamedSymbolRef::declaredField(fieldSym, bind.bind.type), trim(bind.loc), false}});
}
} else {
ENFORCE(false, "Should've handled all cases of variant earlier");
}
continue;
}
if (sym.isFieldOrStaticField()) {
Expand Down Expand Up @@ -1019,9 +1118,18 @@ class CFGTraversal final {
} else {
uint32_t localId = this->functionLocals[localRef];
auto it = this->localDefinitionType.find(localId);
ENFORCE(it != this->localDefinitionType.end(), "file:{}, code:\n{}\naliasMap: {}\n", file.data(gs).path(),
core::Loc(file, loc).toString(gs), this->aliasMap.showRaw(gs, file, cfg));
auto overrideType = computeOverrideType(it->second, type);
optional<core::TypePtr> overrideType;
if (it != this->localDefinitionType.end()) {
overrideType = computeOverrideType(it->second, type);
} else {
LOG_DEBUG(
gs, core::Loc(file, loc),
fmt::format(
"failed to find type info; are you using a code pattern unsupported by Sorbet?\ndebugging "
"information: aliasMap: {}",
this->aliasMap.showRaw(gs, file, cfg)));
overrideType = type;
}
if (isDefinition) {
status = this->scipState.saveDefinition(gs, file, OwnedLocal{this->ctx.owner, localId, loc}, type);
} else {
Expand Down
Loading