From 72dbf22a39d851e7ca5fbaa6b521b27c66f279e6 Mon Sep 17 00:00:00 2001 From: Varun Gandhi Date: Wed, 3 Aug 2022 13:38:36 +0800 Subject: [PATCH] fix: Fix ENFORCE by allowing declared non-static fields. Triggered when trying to index Homebrew/brew. --- scip_indexer/SCIPIndexer.cc | 36 ++++--- test/scip/testdata/inheritance.rb | 49 +++++++++ test/scip/testdata/inheritance.snapshot.rb | 117 +++++++++++++++++++++ 3 files changed, 190 insertions(+), 12 deletions(-) create mode 100644 test/scip/testdata/inheritance.rb create mode 100644 test/scip/testdata/inheritance.snapshot.rb diff --git a/scip_indexer/SCIPIndexer.cc b/scip_indexer/SCIPIndexer.cc index 18dfd68332..18d8fe05d4 100644 --- a/scip_indexer/SCIPIndexer.cc +++ b/scip_indexer/SCIPIndexer.cc @@ -179,7 +179,7 @@ class NamedSymbolRef final { enum class Kind { ClassOrModule, UndeclaredField, - StaticField, + DeclaredField, Method, }; @@ -191,7 +191,7 @@ class NamedSymbolRef final { ENFORCE(s.isClassOrModule()); ENFORCE(!n.exists()); return; - case Kind::StaticField: + case Kind::DeclaredField: ENFORCE(s.isFieldOrStaticField()); ENFORCE(!n.exists()); return; @@ -226,8 +226,8 @@ class NamedSymbolRef final { return NamedSymbolRef(owner, name, type, Kind::UndeclaredField); } - static NamedSymbolRef staticField(core::SymbolRef self, core::TypePtr type) { - return NamedSymbolRef(self, {}, type, Kind::StaticField); + static NamedSymbolRef declaredField(core::SymbolRef self, core::TypePtr type) { + return NamedSymbolRef(self, {}, type, Kind::DeclaredField); } static NamedSymbolRef method(core::SymbolRef self) { @@ -239,7 +239,7 @@ class NamedSymbolRef final { return Kind::UndeclaredField; } if (this->selfOrOwner.isFieldOrStaticField()) { - return Kind::StaticField; + return Kind::DeclaredField; } if (this->selfOrOwner.isMethod()) { return Kind::Method; @@ -252,8 +252,8 @@ class NamedSymbolRef final { case Kind::UndeclaredField: return fmt::format("UndeclaredField(owner: {}, name: {})", this->selfOrOwner.showFullName(gs), this->name.toString(gs)); - case Kind::StaticField: - return fmt::format("StaticField {}", this->selfOrOwner.showFullName(gs)); + case Kind::DeclaredField: + return fmt::format("DeclaredField {}", this->selfOrOwner.showFullName(gs)); case Kind::ClassOrModule: return fmt::format("ClassOrModule {}", this->selfOrOwner.showFullName(gs)); case Kind::Method: @@ -280,7 +280,7 @@ class NamedSymbolRef final { markdown = fmt::format("{} = T.let(_, {})", name, fieldType.show(gs)); break; } - case Kind::StaticField: { + case Kind::DeclaredField: { auto fieldRef = this->selfOrOwner.asFieldRef(); auto name = fieldRef.showFullName(gs); CHECK_TYPE(fieldType, name); @@ -669,7 +669,7 @@ class SCIPState { case Kind::Method: break; case Kind::UndeclaredField: - case Kind::StaticField: + case Kind::DeclaredField: if (overrideType.has_value()) { overrideDocs = symRef.docStrings(gs, overrideType.value(), core::Loc(file, occLoc)); } @@ -760,10 +760,11 @@ class AliasMap final { {NamedSymbolRef::undeclaredField(klass, instr->name, bind.bind.type), bind.loc, false}}); continue; } - if (sym.isStaticField(gs)) { + if (sym.isFieldOrStaticField()) { + ENFORCE(!bind.loc.empty()); this->map.insert( {bind.bind.variable, - {NamedSymbolRef::staticField(instr->what, bind.bind.type), trim(bind.loc), false}}); + {NamedSymbolRef::declaredField(instr->what, bind.bind.type), trim(bind.loc), false}}); continue; } // Outside of definition contexts for classes & modules, @@ -799,6 +800,16 @@ class AliasMap final { return {{namedSym, loc}}; } + string showRaw(const core::GlobalState &gs, core::FileRef file, const cfg::CFG &cfg) const { + return map_to_string(this->map, [&](const cfg::LocalRef &local, auto &data) -> string { + auto symRef = get<0>(data); + auto offsets = get<1>(data); + auto emitted = get<2>(data); + return fmt::format("(local: {}) -> (symRef: {}, emitted: {}, loc: {})", local.toString(gs, cfg), + symRef.showRaw(gs), emitted ? "true" : "false", core::Loc(file, offsets).showRaw(gs)); + }); + } + void extract(Impl &out) { out = std::move(this->map); } @@ -947,7 +958,8 @@ class CFGTraversal final { } else { uint32_t localId = this->functionLocals[localRef]; auto it = this->localDefinitionType.find(localId); - ENFORCE(it != this->localDefinitionType.end()); + 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); if (isDefinition) { status = this->scipState.saveDefinition(gs, file, OwnedLocal{this->ctx.owner, localId, loc}, type); diff --git a/test/scip/testdata/inheritance.rb b/test/scip/testdata/inheritance.rb new file mode 100644 index 0000000000..ccbd2c8ecd --- /dev/null +++ b/test/scip/testdata/inheritance.rb @@ -0,0 +1,49 @@ +# typed: true + +class Z1 + extend T::Sig + + sig { params(a: T::Boolean).void } + def write_f(a) + @f = a + end + + sig { returns(T::Boolean) } + def read_f? + @f + end +end + +class Z2 + extend T::Sig + + sig { returns(T::Boolean) } + def read_f? + @f + end + + sig { params(a: T::Boolean).void } + def write_f(a) + @f = a + end +end + +class Z3 < Z1 + extend T::Sig + + sig { returns(T::Boolean) } + def read_f_plus_1? + @f + 1 + end +end + +class Z4 < Z3 + extend T::Sig + + sig { params(a: T::Boolean).void } + def write_f_plus_1(a) + write_f(a) + @f = read_f_plus_1? + end +end + diff --git a/test/scip/testdata/inheritance.snapshot.rb b/test/scip/testdata/inheritance.snapshot.rb new file mode 100644 index 0000000000..32b87f034b --- /dev/null +++ b/test/scip/testdata/inheritance.snapshot.rb @@ -0,0 +1,117 @@ + # typed: true + + class Z1 +# ^^ definition [..] Z1# + extend T::Sig +# ^ reference [..] T# +# ^^^ reference [..] T#Sig# + + sig { params(a: T::Boolean).void } +# ^^^ reference [..] Sorbet#Private##sig(). +# ^^^^^^ reference [..] T#Private#Methods#DeclBuilder#params(). +# ^ reference [..] T# +# ^^^^^^^ reference [..] T#Boolean. +# ^^^^^^^^^^^^^^^ reference [..] Sorbet#Private#Static# +# ^^^^ reference [..] T#Private#Methods#DeclBuilder#void(). + def write_f(a) +# ^^^^^^^^^^^^^^ definition [..] Z1#write_f(). +# ^ definition local 1~#1000661517 + @f = a +# ^^ definition [..] Z1#@f. +# ^^^^^^ reference [..] Z1#@f. +# ^ reference local 1~#1000661517 + end + + sig { returns(T::Boolean) } +# ^^^ reference [..] Sorbet#Private##sig(). +# ^^^^^^^ reference [..] T#Private#Methods#DeclBuilder#returns(). +# ^ reference [..] T# +# ^^^^^^^ reference [..] T#Boolean. +# ^^^^^^^^^^ reference [..] Sorbet#Private#Static# + def read_f? +# ^^^^^^^^^^^ definition [..] Z1#read_f?(). + @f +# ^^ reference [..] Z1#@f. + end + end + + class Z2 +# ^^ definition [..] Z2# + extend T::Sig +# ^ reference [..] T# +# ^^^ reference [..] T#Sig# + + sig { returns(T::Boolean) } +# ^^^ reference [..] Sorbet#Private##sig(). +# ^^^^^^^ reference [..] T#Private#Methods#DeclBuilder#returns(). +# ^ reference [..] T# +# ^^^^^^^ reference [..] T#Boolean. +# ^^^^^^^^^^ reference [..] Sorbet#Private#Static# + def read_f? +# ^^^^^^^^^^^ definition [..] Z2#read_f?(). + @f +# ^^ reference [..] Z2#@f. + end + + sig { params(a: T::Boolean).void } +# ^^^ reference [..] Sorbet#Private##sig(). +# ^^^^^^ reference [..] T#Private#Methods#DeclBuilder#params(). +# ^ reference [..] T# +# ^^^^^^^ reference [..] T#Boolean. +# ^^^^^^^^^^^^^^^ reference [..] Sorbet#Private#Static# +# ^^^^ reference [..] T#Private#Methods#DeclBuilder#void(). + def write_f(a) +# ^^^^^^^^^^^^^^ definition [..] Z2#write_f(). +# ^ definition local 1~#1000661517 + @f = a +# ^^ definition [..] Z2#@f. +# ^^^^^^ reference [..] Z2#@f. +# ^ reference local 1~#1000661517 + end + end + + class Z3 < Z1 +# ^^ definition [..] Z3# +# ^^ definition [..] Z1# + extend T::Sig +# ^ reference [..] T# +# ^^^ reference [..] T#Sig# + + sig { returns(T::Boolean) } +# ^^^ reference [..] Sorbet#Private##sig(). +# ^^^^^^^ reference [..] T#Private#Methods#DeclBuilder#returns(). +# ^ reference [..] T# +# ^^^^^^^ reference [..] T#Boolean. +# ^^^^^^^^^^ reference [..] Sorbet#Private#Static# + def read_f_plus_1? +# ^^^^^^^^^^^^^^^^^^ definition [..] Z3#read_f_plus_1?(). + @f + 1 +# ^^ reference [..] Z3#@f. + end + end + + class Z4 < Z3 +# ^^ definition [..] Z4# +# ^^ definition [..] Z3# + extend T::Sig +# ^ reference [..] T# +# ^^^ reference [..] T#Sig# + + sig { params(a: T::Boolean).void } +# ^^^ reference [..] Sorbet#Private##sig(). +# ^^^^^^ reference [..] T#Private#Methods#DeclBuilder#params(). +# ^ reference [..] T# +# ^^^^^^^ reference [..] T#Boolean. +# ^^^^^^^^^^^^^^^ reference [..] Sorbet#Private#Static# +# ^^^^ reference [..] T#Private#Methods#DeclBuilder#void(). + def write_f_plus_1(a) +# ^^^^^^^^^^^^^^^^^^^^^ definition [..] Z4#write_f_plus_1(). +# ^ definition local 1~#3337417690 + write_f(a) +# ^ reference local 1~#3337417690 + @f = read_f_plus_1? +# ^^ definition [..] Z4#@f. +# ^^^^^^^^^^^^^^^^^^^ reference [..] Z4#@f. + end + end +