| commit | c9e25ff6c75a0b9501a6f5b2c6c5077f2e84f5af | |
| author | Jeff Hemphill <[email protected]> | |
| Wed, 8 Dec 2021 02:04:23 +0000 (7 18:04 -0800) | ||
| committer | Facebook GitHub Bot <[email protected]> | |
| Wed, 8 Dec 2021 02:06:31 +0000 (7 18:06 -0800) | ||
| tree | 597d4cb1cc1945c70ed4daa3eb786e69fe5cc73c | treesnapshot (tar.gz zip) |
| parent | 7033e7a5a72aa651c23ac7ee443a4725c4420e05 | commitdiff |
Correctly get and fill the attributes of type aliases
Summary:
We had an issue where type alias attribute queries appeared to sometimes be "sticky". Sometimes, when you start up HHVM and ask for the attributes of a type alias, you always get the same answer that had been true when HHVM started, even if the type alias had changed since we started HHVM.
The attributes aren't always sticky, and the bug reproduces somewhat nondeterministically. There were three places where we mixed types and type aliases up, and these bugs sometimes collaborated to produce correct behavior.
1. When we asked for the attributes of a type alias, we would mistakenly query the in-memory table for *type* attributes
1. When we query the in-memory table for type attributes, we find that the type *alias* never exists in that table. Changing the file changes the type alias table correctly, but we're never actually looking at that type alias table.
1. Now the reason we sometimes return correct results, instead of always returning empty results - the DB doesn't distinguish between types and type aliases in the same way as the in-memory map. We use the same function to query the DB for the attributes of either a type or type alias. We then put the DB's answer inside the in-memory table, even if the answer shouldn't go in that table.
This diff has three high-level fixes:
1. This diff makes sure we actually query the correct in-memory table. This is covered by two new queries in the `facts.php` test.
1. This diff also adds new checks to our DB filling code. These checks ensure that we don't pollute a type-alias table with data from the DB that corresponds to types, or vice versa. This is covered by a new test in `symbol-map-test.cpp`.
1. This diff also moves a side effect out of an `always_assert()`. This has no effect, but I was seeing that the production version of HHVM was behaving differently from my debug version of HHVM, I was paranoid about side effects in an assert being the potential problem, and I'm unlikely to fix this style issue in its own diff.
Reviewed By: scotthovestadt
Differential Revision: D32929161
fbshipit-source-id: 5f9542a950cf12cd9c1353b64301461413c044d0
Summary:
We had an issue where type alias attribute queries appeared to sometimes be "sticky". Sometimes, when you start up HHVM and ask for the attributes of a type alias, you always get the same answer that had been true when HHVM started, even if the type alias had changed since we started HHVM.
The attributes aren't always sticky, and the bug reproduces somewhat nondeterministically. There were three places where we mixed types and type aliases up, and these bugs sometimes collaborated to produce correct behavior.
1. When we asked for the attributes of a type alias, we would mistakenly query the in-memory table for *type* attributes
1. When we query the in-memory table for type attributes, we find that the type *alias* never exists in that table. Changing the file changes the type alias table correctly, but we're never actually looking at that type alias table.
1. Now the reason we sometimes return correct results, instead of always returning empty results - the DB doesn't distinguish between types and type aliases in the same way as the in-memory map. We use the same function to query the DB for the attributes of either a type or type alias. We then put the DB's answer inside the in-memory table, even if the answer shouldn't go in that table.
This diff has three high-level fixes:
1. This diff makes sure we actually query the correct in-memory table. This is covered by two new queries in the `facts.php` test.
1. This diff also adds new checks to our DB filling code. These checks ensure that we don't pollute a type-alias table with data from the DB that corresponds to types, or vice versa. This is covered by a new test in `symbol-map-test.cpp`.
1. This diff also moves a side effect out of an `always_assert()`. This has no effect, but I was seeing that the production version of HHVM was behaving differently from my debug version of HHVM, I was paranoid about side effects in an assert being the potential problem, and I'm unlikely to fix this style issue in its own diff.
Reviewed By: scotthovestadt
Differential Revision: D32929161
fbshipit-source-id: 5f9542a950cf12cd9c1353b64301461413c044d0