| commit | a1bf32abca0b06fc6fee91e9affdbe5f90e55845 | |
| author | Jez Ng <jezng@fb.com> | |
| Tue, 1 Mar 2016 14:32:13 +0000 (1 06:32 -0800) | ||
| committer | Hhvm Bot <hhvm-bot-bot@fb.com> | |
| Tue, 1 Mar 2016 15:00:41 +0000 (1 07:00 -0800) | ||
| tree | d69cc61eaa0d08c32ca7dfb6b6f56a2679af131d | treesnapshot (tar.gz zip) |
| parent | 2c844270788b4f3604af7a8567624b0e82d72e81 | commitdiff |
Remove dispatch_infer_ty_hook from make_param_ty
Summary:{D2947924} split make_param_ty into two, and in the process revealed that we
were doing some work twice. In particular, we would call dispatch_infer_ty_hook
in both the param decl step and the param localization step. This seems
redundant -- we would end up dispatching twice for a given param at a given
position, giving a similar type each time (albeit one being localized and the
other not).
From what I can tell, the IDE / Phabricator services that use this feature deal
with multiple types at a given positioning by only keeping the latest. E.g.if
we called the hook at some position `p` with Tarray first and then Tarraykind
later, the hooks would just remember that `p` maps to `Tarraykind`. As such,
skipping the first (decl) call to the hook and keeping just the locl call
should preserve semantics.
The dumpsymbolinfo test seems to bear this out; removing the decl-phase hook
invocation has no effect on it, but removing the locl-phase hook invocation
does result in a unit test failure.
Pretty sure this is safe but adding the original authors of the infer hook
stuff as reviewers just so they have a heads up.
This is part of my split-decl-from-typing (lazy decl) refactoring.
Reviewed By: joelbeales
Differential Revision: D2968551
fb-gh-sync-id: 612882bbb98c4c5d59f2c6900072638a91f0d01c
shipit-source-id: 612882bbb98c4c5d59f2c6900072638a91f0d01c
Summary:{D2947924} split make_param_ty into two, and in the process revealed that we
were doing some work twice. In particular, we would call dispatch_infer_ty_hook
in both the param decl step and the param localization step. This seems
redundant -- we would end up dispatching twice for a given param at a given
position, giving a similar type each time (albeit one being localized and the
other not).
From what I can tell, the IDE / Phabricator services that use this feature deal
with multiple types at a given positioning by only keeping the latest. E.g.if
we called the hook at some position `p` with Tarray first and then Tarraykind
later, the hooks would just remember that `p` maps to `Tarraykind`. As such,
skipping the first (decl) call to the hook and keeping just the locl call
should preserve semantics.
The dumpsymbolinfo test seems to bear this out; removing the decl-phase hook
invocation has no effect on it, but removing the locl-phase hook invocation
does result in a unit test failure.
Pretty sure this is safe but adding the original authors of the infer hook
stuff as reviewers just so they have a heads up.
This is part of my split-decl-from-typing (lazy decl) refactoring.
Reviewed By: joelbeales
Differential Revision: D2968551
fb-gh-sync-id: 612882bbb98c4c5d59f2c6900072638a91f0d01c
shipit-source-id: 612882bbb98c4c5d59f2c6900072638a91f0d01c