Skip to content

Commit 2be2a60

Browse files
fix: Fix occurrences for field inheritance.
Handles: - Instance variables - Class variables - Class instance variables Does not handle modules/mix-ins.
1 parent fbabd1e commit 2be2a60

File tree

10 files changed

+626
-27
lines changed

10 files changed

+626
-27
lines changed

core/GlobalState.cc

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1962,6 +1962,7 @@ unique_ptr<GlobalState> GlobalState::deepCopy(bool keepId) const {
19621962
result->sleepInSlowPathSeconds = this->sleepInSlowPathSeconds;
19631963
result->requiresAncestorEnabled = this->requiresAncestorEnabled;
19641964
result->lspExperimentalFastPathEnabled = this->lspExperimentalFastPathEnabled;
1965+
result->isSCIPRuby = this->isSCIPRuby;
19651966

19661967
if (keepId) {
19671968
result->globalStateId = this->globalStateId;
@@ -2055,6 +2056,7 @@ unique_ptr<GlobalState> GlobalState::copyForIndex() const {
20552056
result->runningUnderAutogen = this->runningUnderAutogen;
20562057
result->censorForSnapshotTests = this->censorForSnapshotTests;
20572058
result->lspExperimentalFastPathEnabled = this->lspExperimentalFastPathEnabled;
2059+
result->isSCIPRuby = this->isSCIPRuby;
20582060
result->sleepInSlowPathSeconds = this->sleepInSlowPathSeconds;
20592061
result->requiresAncestorEnabled = this->requiresAncestorEnabled;
20602062
result->kvstoreUuid = this->kvstoreUuid;

core/GlobalState.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -291,6 +291,13 @@ class GlobalState final {
291291
// If 'true', enable the experimental, symbol-deletion-based fast path mode
292292
bool lspExperimentalFastPathEnabled = false;
293293

294+
// If 'true', we're running in scip-ruby mode.
295+
bool isSCIPRuby = true;
296+
297+
// --- begin scip-ruby specific state
298+
UnorderedMap<core::ClassOrModuleRef, UnorderedSet<core::NameRef>> unresolvedFields;
299+
// --- end scip-ruby specific state
300+
294301
// When present, this indicates that single-package rbi generation is being performed, and contains metadata about
295302
// the packages that are imported by the one whose interface is being generated.
296303
std::optional<packages::ImportInfo> singlePackageImports;

resolver/resolver.cc

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3924,6 +3924,93 @@ class ResolveSignaturesWalk {
39243924
}
39253925
};
39263926

3927+
/// Helper type to traverse ASTs and aggregate information about unresolved fields.
3928+
///
3929+
/// For perf, it would make sense to fuse this along with some other map-reduce
3930+
/// operation over files, but I've kept it separate for now for ease of maintenance.
3931+
class CollectUnresolvedFieldsWalk final {
3932+
UnorderedMap<core::ClassOrModuleRef, UnorderedSet<core::NameRef>> unresolvedFields;
3933+
3934+
public:
3935+
void postTransformUnresolvedIdent(core::Context ctx, ast::ExpressionPtr &tree) {
3936+
auto &unresolvedIdent = ast::cast_tree_nonnull<ast::UnresolvedIdent>(tree);
3937+
using Kind = ast::UnresolvedIdent::Kind;
3938+
core::ClassOrModuleRef klass;
3939+
switch (unresolvedIdent.kind) {
3940+
case Kind::Global:
3941+
case Kind::Local:
3942+
return;
3943+
case Kind::Class: {
3944+
auto enclosingClass = ctx.owner.enclosingClass(ctx);
3945+
klass = enclosingClass.data(ctx)->isSingletonClass(ctx)
3946+
? enclosingClass // in <static-init>
3947+
: enclosingClass.data(ctx)->lookupSingletonClass(ctx); // in static methods
3948+
break;
3949+
}
3950+
case Kind::Instance: {
3951+
klass = ctx.owner.enclosingClass(ctx);
3952+
}
3953+
}
3954+
this->unresolvedFields[klass].insert(unresolvedIdent.name);
3955+
}
3956+
3957+
struct CollectWalkResult {
3958+
UnorderedMap<core::ClassOrModuleRef, UnorderedSet<core::NameRef>> unresolvedFields;
3959+
vector<ast::ParsedFile> trees;
3960+
};
3961+
3962+
static vector<ast::ParsedFile> collect(core::GlobalState &gs, vector<ast::ParsedFile> trees, WorkerPool &workers) {
3963+
Timer timeit(gs.tracer(), "resolver.collect_unresolved_fields");
3964+
const core::GlobalState &igs = gs;
3965+
auto resultq = make_shared<BlockingBoundedQueue<CollectWalkResult>>(trees.size());
3966+
auto fileq = make_shared<ConcurrentBoundedQueue<ast::ParsedFile>>(trees.size());
3967+
for (auto &tree : trees) {
3968+
fileq->push(move(tree), 1);
3969+
}
3970+
trees.clear();
3971+
3972+
workers.multiplexJob("collectUnresolvedFieldsWalk", [&igs, fileq, resultq]() {
3973+
Timer timeit(igs.tracer(), "CollectUnresolvedFieldsWorker");
3974+
CollectUnresolvedFieldsWalk collect;
3975+
CollectWalkResult walkResult;
3976+
vector<ast::ParsedFile> collectedTrees;
3977+
ast::ParsedFile job;
3978+
for (auto result = fileq->try_pop(job); !result.done(); result = fileq->try_pop(job)) {
3979+
if (!result.gotItem()) {
3980+
continue;
3981+
}
3982+
core::Context ictx(igs, core::Symbols::root(), job.file);
3983+
ast::TreeWalk::apply(ictx, collect, job.tree);
3984+
collectedTrees.emplace_back(move(job));
3985+
}
3986+
if (!collectedTrees.empty()) {
3987+
walkResult.trees = move(collectedTrees);
3988+
walkResult.unresolvedFields = std::move(collect.unresolvedFields);
3989+
resultq->push(move(walkResult), walkResult.trees.size());
3990+
}
3991+
});
3992+
3993+
{
3994+
CollectWalkResult threadResult;
3995+
for (auto result = resultq->wait_pop_timed(threadResult, WorkerPool::BLOCK_INTERVAL(), gs.tracer());
3996+
!result.done();
3997+
result = resultq->wait_pop_timed(threadResult, WorkerPool::BLOCK_INTERVAL(), gs.tracer())) {
3998+
if (!result.gotItem()) {
3999+
continue;
4000+
}
4001+
trees.insert(trees.end(), make_move_iterator(threadResult.trees.begin()),
4002+
make_move_iterator(threadResult.trees.end()));
4003+
gs.unresolvedFields.reserve(gs.unresolvedFields.size() + threadResult.unresolvedFields.size());
4004+
gs.unresolvedFields.insert(make_move_iterator(threadResult.unresolvedFields.begin()),
4005+
make_move_iterator(threadResult.unresolvedFields.end()));
4006+
}
4007+
}
4008+
4009+
fast_sort(trees, [](const auto &lhs, const auto &rhs) -> bool { return lhs.file < rhs.file; });
4010+
return trees;
4011+
}
4012+
};
4013+
39274014
class ResolveSanityCheckWalk {
39284015
public:
39294016
void postTransformClassDef(core::Context ctx, ast::ExpressionPtr &tree) {
@@ -4102,6 +4189,9 @@ ast::ParsedFilesOrCancelled Resolver::run(core::GlobalState &gs, vector<ast::Par
41024189

41034190
auto result = resolveSigs(gs, std::move(rtmafResult.trees), workers);
41044191
ResolveTypeMembersAndFieldsWalk::resolvePendingCastItems(gs, rtmafResult.todoResolveCastItems);
4192+
if (gs.isSCIPRuby) {
4193+
result = CollectUnresolvedFieldsWalk::collect(gs, std::move(result), workers);
4194+
}
41054195
sanityCheck(gs, result);
41064196

41074197
return result;

scip_indexer/SCIPIndexer.cc

Lines changed: 114 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
#include "cfg/CFG.h"
2525
#include "common/common.h"
2626
#include "common/sort.h"
27+
#include "core/Error.h"
2728
#include "core/ErrorQueue.h"
2829
#include "core/Loc.h"
2930
#include "core/SymbolRef.h"
@@ -57,6 +58,29 @@ static uint32_t fnv1a_32(const string &s) {
5758

5859
namespace sorbet::scip_indexer {
5960

61+
constexpr sorbet::core::ErrorClass SCIPRubyDebug{400, sorbet::core::StrictLevel::False};
62+
63+
void _log_debug(const sorbet::core::GlobalState &gs, sorbet::core::Loc loc, std::string s) {
64+
if (auto e = gs.beginError(loc, SCIPRubyDebug)) {
65+
auto lines = absl::StrSplit(s, '\n');
66+
for (auto line = lines.begin(); line != lines.end(); line++) {
67+
auto text = string(line->begin(), line->length());
68+
if (line == lines.begin()) {
69+
e.setHeader("[scip-ruby] {}", text);
70+
} else {
71+
e.addErrorNote("{}", text);
72+
}
73+
}
74+
}
75+
}
76+
77+
#ifndef NDEBUG
78+
#define LOG_DEBUG(__gs, __loc, __s) _log_debug(__gs, __loc, __s)
79+
#else
80+
#define LOG_DEBUG(__gs, __s) \
81+
{}
82+
#endif
83+
6084
// TODO(varun): This is an inline workaround for https://github.com/sorbet/sorbet/issues/5925
6185
// I've not changed the main definition because I didn't bother to rerun the tests with the change.
6286
static bool isTemporary(const core::GlobalState &gs, const core::LocalVariable &var) {
@@ -778,6 +802,63 @@ string format_ancestry(const core::GlobalState &gs, core::SymbolRef sym) {
778802
return out.str();
779803
}
780804

805+
static absl::variant</*owner*/ core::ClassOrModuleRef, core::SymbolRef>
806+
findUnresolvedFieldTransitive(const core::GlobalState &gs, core::Loc loc, core::ClassOrModuleRef start,
807+
core::NameRef field) {
808+
auto fieldText = field.shortName(gs);
809+
auto isInstanceVar = fieldText.size() >= 2 && fieldText[0] == '@' && fieldText[1] != '@';
810+
auto isClassInstanceVar = isInstanceVar && start.data(gs)->isSingletonClass(gs);
811+
// Class instance variables are not inherited, unlike ordinary instance
812+
// variables or class variables.
813+
if (isClassInstanceVar) {
814+
return start;
815+
}
816+
auto isClassVar = fieldText.size() >= 2 && fieldText[0] == '@' && fieldText[1] == '@';
817+
if (isClassVar && !start.data(gs)->isSingletonClass(gs)) {
818+
// Triggered when undeclared class variables are accessed from instance methods.
819+
start = start.data(gs)->lookupSingletonClass(gs);
820+
}
821+
822+
// TODO(varun): Should we add a cache here? It seems wasteful to redo
823+
// work for every occurrence.
824+
if (gs.unresolvedFields.find(start) == gs.unresolvedFields.end() ||
825+
!gs.unresolvedFields.find(start)->second.contains(field)) {
826+
// Triggered by code patterns like:
827+
// # top-level
828+
// def MyClass.method
829+
// # blah
830+
// end
831+
// which is not supported by Sorbet.
832+
LOG_DEBUG(gs, loc,
833+
fmt::format("couldn't find field {} in class {};\n"
834+
"are you using a code pattern like def MyClass.method which is unsupported by Sorbet?",
835+
field.exists() ? field.toString(gs) : "<non-existent>",
836+
start.exists() ? start.showFullName(gs) : "<non-existent>"));
837+
// As a best-effort guess, assume that the definition is
838+
// in this class but we somehow missed it.
839+
return start;
840+
}
841+
842+
auto best = start;
843+
auto cur = start;
844+
while (cur.exists()) {
845+
auto klass = cur.data(gs);
846+
auto sym = klass->findMember(gs, field);
847+
if (sym.exists()) {
848+
return sym;
849+
}
850+
auto it = gs.unresolvedFields.find(cur);
851+
if (it != gs.unresolvedFields.end() && it->second.contains(field)) {
852+
best = cur;
853+
}
854+
if (cur == klass->superClass()) { // FIXME(varun): Handle mix-ins
855+
break;
856+
}
857+
cur = klass->superClass();
858+
}
859+
return best;
860+
}
861+
781862
// Loosely inspired by AliasesAndKeywords in IREmitterContext.cc
782863
class AliasMap final {
783864
public:
@@ -816,9 +897,27 @@ class AliasMap final {
816897
}
817898
if (sym == core::Symbols::Magic_undeclaredFieldStub()) {
818899
ENFORCE(!bind.loc.empty());
819-
this->map.insert( // no trim(...) because undeclared fields shouldn't have ::
820-
{bind.bind.variable,
821-
{NamedSymbolRef::undeclaredField(klass, instr->name, bind.bind.type), bind.loc, false}});
900+
ENFORCE(klass.isClassOrModule());
901+
auto result = findUnresolvedFieldTransitive(ctx, ctx.locAt(bind.loc), klass.asClassOrModuleRef(),
902+
instr->name);
903+
if (absl::holds_alternative<core::ClassOrModuleRef>(result)) {
904+
auto klass = absl::get<core::ClassOrModuleRef>(result);
905+
if (klass.exists()) {
906+
this->map.insert( // no trim(...) because undeclared fields shouldn't have ::
907+
{bind.bind.variable,
908+
{NamedSymbolRef::undeclaredField(klass, instr->name, bind.bind.type), bind.loc,
909+
false}});
910+
}
911+
} else if (absl::holds_alternative<core::SymbolRef>(result)) {
912+
auto fieldSym = absl::get<core::SymbolRef>(result);
913+
if (fieldSym.exists()) {
914+
this->map.insert(
915+
{bind.bind.variable,
916+
{NamedSymbolRef::declaredField(fieldSym, bind.bind.type), trim(bind.loc), false}});
917+
}
918+
} else {
919+
ENFORCE(false, "Should've handled all cases of variant earlier");
920+
}
822921
continue;
823922
}
824923
if (sym.isFieldOrStaticField()) {
@@ -1019,9 +1118,18 @@ class CFGTraversal final {
10191118
} else {
10201119
uint32_t localId = this->functionLocals[localRef];
10211120
auto it = this->localDefinitionType.find(localId);
1022-
ENFORCE(it != this->localDefinitionType.end(), "file:{}, code:\n{}\naliasMap: {}\n", file.data(gs).path(),
1023-
core::Loc(file, loc).toString(gs), this->aliasMap.showRaw(gs, file, cfg));
1024-
auto overrideType = computeOverrideType(it->second, type);
1121+
optional<core::TypePtr> overrideType;
1122+
if (it != this->localDefinitionType.end()) {
1123+
overrideType = computeOverrideType(it->second, type);
1124+
} else {
1125+
LOG_DEBUG(
1126+
gs, core::Loc(file, loc),
1127+
fmt::format(
1128+
"failed to find type info; are you using a code pattern unsupported by Sorbet?\ndebugging "
1129+
"information: aliasMap: {}",
1130+
this->aliasMap.showRaw(gs, file, cfg)));
1131+
overrideType = type;
1132+
}
10251133
if (isDefinition) {
10261134
status = this->scipState.saveDefinition(gs, file, OwnedLocal{this->ctx.owner, localId, loc}, type);
10271135
} else {

0 commit comments

Comments
 (0)