From 08bc65eb768a0c96f741e9264c052c675de763bf Mon Sep 17 00:00:00 2001 From: Varun Gandhi Date: Wed, 29 Jun 2022 18:51:13 +0200 Subject: [PATCH] feat: Add support for function arguments. This includes support for array indexing and hash lookups. --- cfg/Instructions.cc | 2 + scip_indexer/SCIPIndexer.cc | 32 +++++++----- test/scip/testdata/syntax.rb | 30 +++++++++-- test/scip/testdata/syntax.rb.snapshot | 74 +++++++++++++++++++++++---- 4 files changed, 111 insertions(+), 27 deletions(-) diff --git a/cfg/Instructions.cc b/cfg/Instructions.cc index 5c1fac67d4..5b22d6165d 100644 --- a/cfg/Instructions.cc +++ b/cfg/Instructions.cc @@ -130,9 +130,11 @@ Send::Send(LocalRef recv, core::LocOffsets receiverLoc, core::NameRef fun, core: args.size()); this->args.resize(args.size()); + ENFORCE(this->args.size() == this->argLocs.size(), "Expected number of args and argLocs to match"); int i = 0; for (const auto &e : args) { this->args[i].variable = e; + this->args[i].loc = this->argLocs[i]; // NOTE(varun): loc copying i++; } categoryCounterInc("cfg", "send"); diff --git a/scip_indexer/SCIPIndexer.cc b/scip_indexer/SCIPIndexer.cc index 3068cad01e..f02bc7ca8b 100644 --- a/scip_indexer/SCIPIndexer.cc +++ b/scip_indexer/SCIPIndexer.cc @@ -518,29 +518,37 @@ class CFGTraversal final { case cfg::Tag::Send: { // emit occurrence for function auto send = cfg::cast_instruction(binding.value); - if (!send->fun.exists()) { - // TODO(varun): when does this happen? - continue; - } + // Emit reference for the receiver, if present. if (send->recv.loc.exists() && !send->recv.loc.empty()) { this->emitLocalOccurrence(cfg, bb, send->recv.occurrence(), ValueCategory::RValue); } // Emit reference for the method being called - if (!isTemporary(gs, core::LocalVariable(send->fun, 1))) { + if (send->fun.exists() && !isTemporary(gs, core::LocalVariable(send->fun, 1))) { // HACK(varun): We should probably add a helper function to check // for names corresponding to temporaries? Making up a fake local // variable seems a little gross. auto funSym = lookupRecursive(gs, method, send->fun); - if (!funSym.exists()) { - print_err("# lookup for fun symbol {} failed\n", send->fun.shortName(gs)); - continue; + if (funSym.exists()) { + ENFORCE(send->funLoc.exists() && !send->funLoc.empty()); + auto status = + this->scipState.saveReference(gs, this->ctx.file, funSym, send->funLoc, 0); + ENFORCE(status.ok()); } - ENFORCE(send->funLoc.exists()); - ENFORCE(!send->funLoc.empty()); - auto status = this->scipState.saveReference(gs, this->ctx.file, funSym, send->funLoc, 0); - ENFORCE_NO_TIMER(status.ok()); + print_err("# lookup for fun symbol {} failed\n", send->fun.shortName(gs)); + } + + // Emit references for arguments + for (auto &arg : send->args) { + // NOTE: For constructs like a += b, the instruction sequence ends up being: + // $tmp = $b + // $a = $tmp.+($b) + // The location for $tmp will point to $a in the source. However, the second one is a read, + // and the first one is a write. Instead of emitting two occurrences, it'd be nice to emit + // a combined read-write occurrence. However, that would require complicating the code a + // bit, so let's leave it as-is for now. + this->emitLocalOccurrence(cfg, bb, arg.occurrence(), ValueCategory::RValue); } break; diff --git a/test/scip/testdata/syntax.rb b/test/scip/testdata/syntax.rb index 51a572d826..58060ab293 100644 --- a/test/scip/testdata/syntax.rb +++ b/test/scip/testdata/syntax.rb @@ -1,10 +1,6 @@ # typed: true -a = [57, 33] -i = 0 -a[i] # a.[](i) -a << 970 # a.push(970) -a[2..-1] +_ = 0 # stub for global def globalFn1() x = 10 @@ -14,3 +10,27 @@ def globalFn1() def globalFn2() x = globalFn1() end + +def args(x, y) + z = x + y + if x == 2 + z += y + else + z += x + end + z +end + +def arrays(a, i) + a[0] = 0 + a[1] = a[2] + a[i] = a[i + 1] + b = a[2..-1] + a << a[-1] +end + +def hashes(h, k) + h["hello"] = "world" + old = h["world"] + h[k] = h[old] +end \ No newline at end of file diff --git a/test/scip/testdata/syntax.rb.snapshot b/test/scip/testdata/syntax.rb.snapshot index 791b198448..5facedb36f 100644 --- a/test/scip/testdata/syntax.rb.snapshot +++ b/test/scip/testdata/syntax.rb.snapshot @@ -1,16 +1,8 @@ # typed: true - a = [57, 33] + _ = 0 # stub for global #^ definition local 1~#119448696 -#^^^^^^^^^^^^^^^ definition scip-ruby gem TODO TODO (). - i = 0 -#^ definition local 2~#119448696 - a[i] # a.[](i) -#^ reference local 1~#119448696 - a << 970 # a.push(970) -#^ reference local 1~#119448696 - a[2..-1] -#^ reference local 1~#119448696 +#^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ definition scip-ruby gem TODO TODO (). def globalFn1() #^^^^^^^^^^^^^^^ definition scip-ruby gem TODO TODO globalFn1(). @@ -27,3 +19,65 @@ # ^^^^^^^^^^^^^^^ reference local 1~#3796204016 # ^^^^^^^^^ reference scip-ruby gem TODO TODO globalFn1(). end + + def args(x, y) +#^^^^^^^^^^^^^^ definition scip-ruby gem TODO TODO args(). +# ^ definition local 1~#2634721084 +# ^ definition local 2~#2634721084 + z = x + y +# ^ definition local 3~#2634721084 +# ^ reference local 1~#2634721084 +# ^ reference local 2~#2634721084 + if x == 2 +# ^ reference local 1~#2634721084 + z += y +# ^ reference local 3~#2634721084 +# ^ reference local 3~#2634721084 +# ^ reference local 2~#2634721084 + else + z += x +# ^ reference local 3~#2634721084 +# ^ reference local 3~#2634721084 +# ^ reference local 1~#2634721084 + end + z +# ^ reference local 3~#2634721084 + end + + def arrays(a, i) +#^^^^^^^^^^^^^^^^ definition scip-ruby gem TODO TODO arrays(). +# ^ definition local 1~#513334479 +# ^ definition local 2~#513334479 + a[0] = 0 +# ^ reference local 1~#513334479 + a[1] = a[2] +# ^ reference local 1~#513334479 +# ^ reference local 1~#513334479 + a[i] = a[i + 1] +# ^ reference local 1~#513334479 +# ^ reference local 2~#513334479 +# ^ reference local 1~#513334479 +# ^ reference local 2~#513334479 + b = a[2..-1] +# ^ definition local 3~#513334479 +# ^ reference local 1~#513334479 + a << a[-1] +# ^ reference local 1~#513334479 +# ^ reference local 1~#513334479 + end + + def hashes(h, k) +#^^^^^^^^^^^^^^^^ definition scip-ruby gem TODO TODO hashes(). +# ^ definition local 1~#1685166589 +# ^ definition local 2~#1685166589 + h["hello"] = "world" +# ^ reference local 1~#1685166589 + old = h["world"] +# ^^^ definition local 3~#1685166589 +# ^ reference local 1~#1685166589 + h[k] = h[old] +# ^ reference local 1~#1685166589 +# ^ reference local 2~#1685166589 +# ^ reference local 1~#1685166589 +# ^^^ reference local 3~#1685166589 + end