Skip to content
This repository has been archived by the owner on Dec 1, 2024. It is now read-only.

Understand calculated width not matching sourceRef width #328

Open
fredemmott opened this issue Oct 23, 2020 · 1 comment
Open

Understand calculated width not matching sourceRef width #328

fredemmott opened this issue Oct 23, 2020 · 1 comment

Comments

@fredemmott
Copy link
Contributor

I wanted to optimize Node::getWidth() by using the $this->sourceRef['width'] when available; this doesn't match:

diff --git a/src/nodes/Node.hack b/src/nodes/Node.hack
index 36135d86..0e7346f5 100644
--- a/src/nodes/Node.hack
+++ b/src/nodes/Node.hack
@@ -103,6 +103,15 @@ abstract class Node implements IMemoizeParam {
         $width += $node->getWidth();
       }
       $this->_width = $width;
+      if ($this->sourceRef) {
+        invariant(
+          $width == $this->sourceRef['width'],
+          "Calculated width %d of node type %s does not match sourceref %d",
+          $width,
+          static::class,
+          $this->sourceRef['width'],
+        );
+      }
       return $width;
     } else {
       return $this->_width;

This gets me:

fredemmott@fredemmott-fbmbp hhast % vendor/bin/hacktest tests/UnusedVariableLinterTest.hack
.................E.................

1) Facebook\HHAST\UnusedVariableLinterTest::testCleanExample with data set #13
In file "/var/folders/9v/sc1xb84516v_0rv47v0ygm340000gn/T//hhast-tmp-eba3ee5a021160c3708cadb177f6ce39.php": Failed to deserialize AST: Calculated width 4 of node type Facebook\HHAST\NodeList does not match sourceref 0
    1 <?hh function foo() { return function(inout vec<int> $p) ==> { $p[] = 1; }; }
     --------------------------------------------------------- ^ HERE

Summary: 35 test(s), 34 passed, 0 failed, 0 skipped, 1 error(s).

All other tests are fine

@fredemmott
Copy link
Contributor Author

If this can be understood and changed so they match, this would allow a straightforward performance optimization

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

1 participant