Closed Atry closed 2 years ago
This pull request was exported from Phabricator. Differential Revision: D37754247
@Atry has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.
This pull request was exported from Phabricator. Differential Revision: D37754247
This pull request was exported from Phabricator. Differential Revision: D37754247
This pull request was exported from Phabricator. Differential Revision: D37754247
This pull request was exported from Phabricator. Differential Revision: D37754247
This pull request was exported from Phabricator. Differential Revision: D37754247
This pull request was exported from Phabricator. Differential Revision: D37754247
This pull request was exported from Phabricator. Differential Revision: D37754247
This pull request was exported from Phabricator. Differential Revision: D37754247
This pull request was exported from Phabricator. Differential Revision: D37754247
This pull request was exported from Phabricator. Differential Revision: D37754247
This pull request was exported from Phabricator. Differential Revision: D37754247
This pull request was exported from Phabricator. Differential Revision: D37754247
This pull request was exported from Phabricator. Differential Revision: D37754247
Summary: Pull Request resolved: https://github.com/facebook/hhvm/pull/9114
Refactor the DeclProvider api to solve some impedence mismatches. The old FFI hardcoded the enum values of AutoloadMap::KindOf, and they have drifted; code accidentally worked but clearly we were on borrowed time. The old API allowed any (NameType,string) query to return any Decl variant, which encodes more entropy than the language even allows. Type queries can only return Class or Typedef, Constant queries can only return Const decls, and so forth. This is fixed in the new API by adding category-specific methods (starting with types) and completely removing the too-loose API. The old API used old-school C conventions: a function pointer and a data pointer. Replace this with just a data handle, and C++ glue code that provides a C++ pure virtual base class, and all the glue is now in hackc/ffi_bridge. C++ clients just extend the pure-virtual base class. There was a pointer punning bug in ExternalDeclProvider that accidentally worked in statically-linked builds but crashed in dynamically linked builds, for unknown reasons. In the past we passed Decls from Rust to C++ with a simple newtype wrapper struct, and used unsafe pointer punning that assume &wrapper == &wrapped_field. Since then the struct had evolved to have multiple named fields, the punning became unsound, but none of it was caught at compile time. Now, as long as ExternalDeclProviderResult is kept in sync then we are ok, and both definitions are encapsulated under hackc/ffi_bridge. ExternalDeclProvider glue code has all moved into hackc/ffi_bridge, for better encapsulation.
Differential Revision: D37754247