Fix instrumentation to allow use of yield statements as expressions >_<

recoilphp · Dec 12, 2016 · 0875b76 · 0875b76
1 parent 568eefa
commit 0875b76
Show file tree

Hide file tree

Showing 14 changed files with 97 additions and 46 deletions.
diff --git a/src/Instrumentation/Instrumentor.php b/src/Instrumentation/Instrumentor.php
@@ -78,19 +78,19 @@ private function instrumentCoroutine(FunctionLike $function)
             return;
         }
 
-        $firstStatement = $statements[0];
-        $function->setAttribute('lastInstrumentedLine', $firstStatement->getLine());
-        $this->consume($firstStatement->getAttribute('startFilePos'));
+        $this->consume($statements[0]->getAttribute('startFilePos'));
 
-        $this->inject(
+        $this->output .= 'assert((';
+        $this->output .= sprintf(
             '(%s = \class_exists(\\%s::class) ? yield \\%s::install() : null) && '
-            . '%s->setCoroutine(__FILE__, __LINE__, __CLASS__, __FUNCTION__, %s, \func_get_args())',
+            . '%s->setCoroutine(__FILE__, __CLASS__, __FUNCTION__, %s, \func_get_args())',
             self::TRACE_VARIABLE_NAME,
             Trace::class,
             Trace::class,
             self::TRACE_VARIABLE_NAME,
             var_export($this->callType($function), true)
         );
+        $this->output .= ') || true); ';
     }
 
     /**
@@ -100,19 +100,16 @@ private function instrumentCoroutine(FunctionLike $function)
      */
     private function instrumentYield(Yield_ $yield)
     {
-        $function = $this->functionStack->top();
-        $startLine = $yield->getLine();
-
-        if ($startLine > $function->getAttribute('lastInstrumentedLine')) {
-            $function->setAttribute('lastInstrumentedLine', $startLine);
-            $this->consume($yield->getAttribute('startFilePos'));
-
-            $this->inject(
-                '%s && %s->setLine(__LINE__)',
-                self::TRACE_VARIABLE_NAME,
-                self::TRACE_VARIABLE_NAME
-            );
-        }
+        $this->consume($yield->getAttribute('startFilePos'));
+        $this->output .= '(!assert((';
+        $this->output .= sprintf(
+            '%s && %s->setLine(__LINE__)',
+            self::TRACE_VARIABLE_NAME,
+            self::TRACE_VARIABLE_NAME
+        );
+        $this->output .= ') || true) ?: ';
+        $this->consume($yield->getAttribute('endFilePos') + 1);
+        $this->output .= ')';
     }
 
     /**
@@ -160,14 +157,6 @@ private function consume(int $position)
         $this->position = $position;
     }
 
-    /**
-     * Injection instrumentation code inside an assertion.
-     */
-    private function inject(string $pattern, string ...$arguments)
-    {
-        $this->output .= 'assert((' . sprintf($pattern, ...$arguments) . ') || true); ';
-    }
-
     /**
      * @access private
      */

diff --git a/src/Instrumentation/Trace.php b/src/Instrumentation/Trace.php
@@ -46,7 +46,6 @@ public function await(Listener $listener)
      */
     public function setCoroutine(
         string $file,
-        int $line,
         string $class,
         string $function,
         string $type,
@@ -55,7 +54,6 @@ public function setCoroutine(
         assert($this->stackDepth > 0);
 
         $this->currentFile = $file;
-        $this->currentLine = $line;
 
         $frame = &$this->stackFrames[$this->stackDepth - 1];
         $frame['function'] = $function;

diff --git a/test/fixture/Instrumentation/Instrumentor/coroutine-anonymous-class.output.php b/test/fixture/Instrumentation/Instrumentor/coroutine-anonymous-class.output.php
@@ -8,13 +8,13 @@
 {
     public static function staticMethod() : Coroutine
     {
-        assert((($μ = \class_exists(\Recoil\Dev\Instrumentation\Trace::class) ? yield \Recoil\Dev\Instrumentation\Trace::install() : null) && $μ->setCoroutine(__FILE__, __LINE__, __CLASS__, __FUNCTION__, '::', \func_get_args())) || true); yield 1;
-        assert(($μ && $μ->setLine(__LINE__)) || true); yield 2;
+        assert((($μ = \class_exists(\Recoil\Dev\Instrumentation\Trace::class) ? yield \Recoil\Dev\Instrumentation\Trace::install() : null) && $μ->setCoroutine(__FILE__, __CLASS__, __FUNCTION__, '::', \func_get_args())) || true); (!assert(($μ && $μ->setLine(__LINE__)) || true) ?: yield 1);
+        (!assert(($μ && $μ->setLine(__LINE__)) || true) ?: yield 2);
     }
 
     public function instanceMethod() : Coroutine
     {
-        assert((($μ = \class_exists(\Recoil\Dev\Instrumentation\Trace::class) ? yield \Recoil\Dev\Instrumentation\Trace::install() : null) && $μ->setCoroutine(__FILE__, __LINE__, __CLASS__, __FUNCTION__, '->', \func_get_args())) || true); yield 1;
-        assert(($μ && $μ->setLine(__LINE__)) || true); yield 2;
+        assert((($μ = \class_exists(\Recoil\Dev\Instrumentation\Trace::class) ? yield \Recoil\Dev\Instrumentation\Trace::install() : null) && $μ->setCoroutine(__FILE__, __CLASS__, __FUNCTION__, '->', \func_get_args())) || true); (!assert(($μ && $μ->setLine(__LINE__)) || true) ?: yield 1);
+        (!assert(($μ && $μ->setLine(__LINE__)) || true) ?: yield 2);
     }
 };
diff --git a/test/fixture/Instrumentation/Instrumentor/coroutine-closure.output.php b/test/fixture/Instrumentation/Instrumentor/coroutine-closure.output.php
@@ -6,6 +6,6 @@
 use Generator as Coroutine;
 
 function () : Coroutine {
-    assert((($μ = \class_exists(\Recoil\Dev\Instrumentation\Trace::class) ? yield \Recoil\Dev\Instrumentation\Trace::install() : null) && $μ->setCoroutine(__FILE__, __LINE__, __CLASS__, __FUNCTION__, '->', \func_get_args())) || true); yield 1;
-    assert(($μ && $μ->setLine(__LINE__)) || true); yield 2;
+    assert((($μ = \class_exists(\Recoil\Dev\Instrumentation\Trace::class) ? yield \Recoil\Dev\Instrumentation\Trace::install() : null) && $μ->setCoroutine(__FILE__, __CLASS__, __FUNCTION__, '->', \func_get_args())) || true); (!assert(($μ && $μ->setLine(__LINE__)) || true) ?: yield 1);
+    (!assert(($μ && $μ->setLine(__LINE__)) || true) ?: yield 2);
 };
diff --git a/test/fixture/Instrumentation/Instrumentor/coroutine-function.output.php b/test/fixture/Instrumentation/Instrumentor/coroutine-function.output.php
@@ -7,6 +7,6 @@
 
 function fn() : Coroutine
 {
-    assert((($μ = \class_exists(\Recoil\Dev\Instrumentation\Trace::class) ? yield \Recoil\Dev\Instrumentation\Trace::install() : null) && $μ->setCoroutine(__FILE__, __LINE__, __CLASS__, __FUNCTION__, '', \func_get_args())) || true); yield 1;
-    assert(($μ && $μ->setLine(__LINE__)) || true); yield 2;
+    assert((($μ = \class_exists(\Recoil\Dev\Instrumentation\Trace::class) ? yield \Recoil\Dev\Instrumentation\Trace::install() : null) && $μ->setCoroutine(__FILE__, __CLASS__, __FUNCTION__, '', \func_get_args())) || true); (!assert(($μ && $μ->setLine(__LINE__)) || true) ?: yield 1);
+    (!assert(($μ && $μ->setLine(__LINE__)) || true) ?: yield 2);
 }
diff --git a/test/fixture/Instrumentation/Instrumentor/coroutine-methods.output.php b/test/fixture/Instrumentation/Instrumentor/coroutine-methods.output.php
@@ -9,13 +9,13 @@ class X
 {
     public static function staticMethod() : Coroutine
     {
-        assert((($μ = \class_exists(\Recoil\Dev\Instrumentation\Trace::class) ? yield \Recoil\Dev\Instrumentation\Trace::install() : null) && $μ->setCoroutine(__FILE__, __LINE__, __CLASS__, __FUNCTION__, '::', \func_get_args())) || true); yield 1;
-        assert(($μ && $μ->setLine(__LINE__)) || true); yield 2;
+        assert((($μ = \class_exists(\Recoil\Dev\Instrumentation\Trace::class) ? yield \Recoil\Dev\Instrumentation\Trace::install() : null) && $μ->setCoroutine(__FILE__, __CLASS__, __FUNCTION__, '::', \func_get_args())) || true); (!assert(($μ && $μ->setLine(__LINE__)) || true) ?: yield 1);
+        (!assert(($μ && $μ->setLine(__LINE__)) || true) ?: yield 2);
     }
 
     public function instanceMethod() : Coroutine
     {
-        assert((($μ = \class_exists(\Recoil\Dev\Instrumentation\Trace::class) ? yield \Recoil\Dev\Instrumentation\Trace::install() : null) && $μ->setCoroutine(__FILE__, __LINE__, __CLASS__, __FUNCTION__, '->', \func_get_args())) || true); yield 1;
-        assert(($μ && $μ->setLine(__LINE__)) || true); yield 2;
+        assert((($μ = \class_exists(\Recoil\Dev\Instrumentation\Trace::class) ? yield \Recoil\Dev\Instrumentation\Trace::install() : null) && $μ->setCoroutine(__FILE__, __CLASS__, __FUNCTION__, '->', \func_get_args())) || true); (!assert(($μ && $μ->setLine(__LINE__)) || true) ?: yield 1);
+        (!assert(($μ && $μ->setLine(__LINE__)) || true) ?: yield 2);
     }
 }
diff --git a/test/fixture/Instrumentation/Instrumentor/coroutine-nested-closure.output.php b/test/fixture/Instrumentation/Instrumentor/coroutine-nested-closure.output.php
@@ -6,10 +6,10 @@
 use Generator as Coroutine;
 
 function () : Coroutine {
-    assert((($μ = \class_exists(\Recoil\Dev\Instrumentation\Trace::class) ? yield \Recoil\Dev\Instrumentation\Trace::install() : null) && $μ->setCoroutine(__FILE__, __LINE__, __CLASS__, __FUNCTION__, '->', \func_get_args())) || true); function () : Coroutine {
-        assert((($μ = \class_exists(\Recoil\Dev\Instrumentation\Trace::class) ? yield \Recoil\Dev\Instrumentation\Trace::install() : null) && $μ->setCoroutine(__FILE__, __LINE__, __CLASS__, __FUNCTION__, '->', \func_get_args())) || true); yield 1;
-        assert(($μ && $μ->setLine(__LINE__)) || true); yield 2;
+    assert((($μ = \class_exists(\Recoil\Dev\Instrumentation\Trace::class) ? yield \Recoil\Dev\Instrumentation\Trace::install() : null) && $μ->setCoroutine(__FILE__, __CLASS__, __FUNCTION__, '->', \func_get_args())) || true); function () : Coroutine {
+        assert((($μ = \class_exists(\Recoil\Dev\Instrumentation\Trace::class) ? yield \Recoil\Dev\Instrumentation\Trace::install() : null) && $μ->setCoroutine(__FILE__, __CLASS__, __FUNCTION__, '->', \func_get_args())) || true); (!assert(($μ && $μ->setLine(__LINE__)) || true) ?: yield 1);
+        (!assert(($μ && $μ->setLine(__LINE__)) || true) ?: yield 2);
     };
 
-    assert(($μ && $μ->setLine(__LINE__)) || true); yield 3;
+    (!assert(($μ && $μ->setLine(__LINE__)) || true) ?: yield 3);
 };
diff --git a/test/fixture/Instrumentation/Instrumentor/hint-is-case-insensitive.output.php b/test/fixture/Instrumentation/Instrumentor/hint-is-case-insensitive.output.php
@@ -7,6 +7,6 @@
 
 function fn() : coroutine
 {
-    assert((($μ = \class_exists(\Recoil\Dev\Instrumentation\Trace::class) ? yield \Recoil\Dev\Instrumentation\Trace::install() : null) && $μ->setCoroutine(__FILE__, __LINE__, __CLASS__, __FUNCTION__, '', \func_get_args())) || true); yield 1;
-    assert(($μ && $μ->setLine(__LINE__)) || true); yield 2;
+    assert((($μ = \class_exists(\Recoil\Dev\Instrumentation\Trace::class) ? yield \Recoil\Dev\Instrumentation\Trace::install() : null) && $μ->setCoroutine(__FILE__, __CLASS__, __FUNCTION__, '', \func_get_args())) || true); (!assert(($μ && $μ->setLine(__LINE__)) || true) ?: yield 1);
+    (!assert(($μ && $μ->setLine(__LINE__)) || true) ?: yield 2);
 }
diff --git a/test/fixture/Instrumentation/Instrumentor/yield-expression.input.php b/test/fixture/Instrumentation/Instrumentor/yield-expression.input.php
@@ -0,0 +1,11 @@
+<?php
+/**
+ * This case verifies yield is instrumented correctly when used as an expression.
+ */
+
+use Generator as Coroutine;
+
+function () : Coroutine {
+    $v = yield 1;
+    return yield 2;
+};
diff --git a/test/fixture/Instrumentation/Instrumentor/yield-expression.output.php b/test/fixture/Instrumentation/Instrumentor/yield-expression.output.php
@@ -0,0 +1,11 @@
+<?php
+/**
+ * This case verifies yield is instrumented correctly when used as an expression.
+ */
+
+use Generator as Coroutine;
+
+function () : Coroutine {
+    assert((($μ = \class_exists(\Recoil\Dev\Instrumentation\Trace::class) ? yield \Recoil\Dev\Instrumentation\Trace::install() : null) && $μ->setCoroutine(__FILE__, __CLASS__, __FUNCTION__, '->', \func_get_args())) || true); $v = (!assert(($μ && $μ->setLine(__LINE__)) || true) ?: yield 1);
+    return (!assert(($μ && $μ->setLine(__LINE__)) || true) ?: yield 2);
+};
diff --git a/test/fixture/Instrumentation/Instrumentor/yield-key.input.php b/test/fixture/Instrumentation/Instrumentor/yield-key.input.php
@@ -0,0 +1,10 @@
+<?php
+/**
+ * This case verifies that yields are instrumented correctly when they have a key.
+ */
+
+use Generator as Coroutine;
+
+function () : Coroutine {
+    yield 1 => 2;
+};
diff --git a/test/fixture/Instrumentation/Instrumentor/yield-key.output.php b/test/fixture/Instrumentation/Instrumentor/yield-key.output.php
@@ -0,0 +1,10 @@
+<?php
+/**
+ * This case verifies that yields are instrumented correctly when they have a key.
+ */
+
+use Generator as Coroutine;
+
+function () : Coroutine {
+    assert((($μ = \class_exists(\Recoil\Dev\Instrumentation\Trace::class) ? yield \Recoil\Dev\Instrumentation\Trace::install() : null) && $μ->setCoroutine(__FILE__, __CLASS__, __FUNCTION__, '->', \func_get_args())) || true); (!assert(($μ && $μ->setLine(__LINE__)) || true) ?: yield 1 => 2);
+};
diff --git a/test/fixture/Instrumentation/Instrumentor/yield-no-value.input.php b/test/fixture/Instrumentation/Instrumentor/yield-no-value.input.php
@@ -0,0 +1,11 @@
+<?php
+/**
+ * This case verifies that yields are instrumented correctly when they do not
+ * have a value.
+ */
+
+use Generator as Coroutine;
+
+function () : Coroutine {
+    yield;
+};
diff --git a/test/fixture/Instrumentation/Instrumentor/yield-no-value.output.php b/test/fixture/Instrumentation/Instrumentor/yield-no-value.output.php
@@ -0,0 +1,11 @@
+<?php
+/**
+ * This case verifies that yields are instrumented correctly when they do not
+ * have a value.
+ */
+
+use Generator as Coroutine;
+
+function () : Coroutine {
+    assert((($μ = \class_exists(\Recoil\Dev\Instrumentation\Trace::class) ? yield \Recoil\Dev\Instrumentation\Trace::install() : null) && $μ->setCoroutine(__FILE__, __CLASS__, __FUNCTION__, '->', \func_get_args())) || true); (!assert(($μ && $μ->setLine(__LINE__)) || true) ?: yield);
+};