Remove internal type from __Sealed attribute in implicit_context.hhi
D74667088 marked MemoSensitiveImplicitContext as __Sealed, with only a single class permitted to directly extend it. This class is however internal to Meta, causing typechecking errors in OSS Hack.
As a fix, mark the attribute with // @oss-disable to remove it during the code export process.
NOTE: If I'm understanding the docs[1] for @oss-disable / @oss-enable correctly, this should be the correct way to make this change, which should then transform into // @oss-disable FBMemoAgnosticImplicitContext::class once this patch is merged and exported. Let me know if this assumption is incorrect.
[1] https://github.com/facebook/buck2/blob/239ab927a5be7dee3035141d29e2e9c91e8ea771/HACKING.md?plain=1#L168
@facebook-github-bot has imported this pull request. If you are a Meta employee, you can view this in D79505213. (Because this pull request was imported automatically, there will not be any future comments.)
Our HSL CI fails on this diff, I presume because you end up with <<__Sealed()>> which doesn't allow extension at all. Perhaps we should just not seal it in OSS?
@mszabo-wikia has updated the pull request. You must reimport the pull request before landing.
@Wilfred Thanks, makes perfect sense. Done.
@mszabo-wikia has updated the pull request. You must reimport the pull request before landing.
Hi there @mszabo-wikia :slightly_smiling_face:,
Thank you for removing references to these internal names. Making sure these fixes land in upstream is important to all and a right thing to do.
I believe there are two small issues with this PR. I have had to remove two references from this commit, but this PR touches only one.
https://github.com/facebook/hhvm/blob/d832512151f8a72759b400e1e86941f0312e5022/hphp/hack/hhi/implicit_context.hhi#L38
https://github.com/facebook/hhvm/blob/d832512151f8a72759b400e1e86941f0312e5022/hphp/hack/hhi/implicit_context.hhi#L52
This diff also changed FBMemoSensitiveImplicitContext to FBMemoAgnosticImplicitContext. Is that intentional?
@mszabo-wikia has updated the pull request. You must reimport the pull request before landing.
@hershel-theodore-layton Thanks, yeah that was meant to go on the other class. Fixed now.
@mszabo-wikia has updated the pull request. You must reimport the pull request before landing.
@mszabo-wikia has updated the pull request. You must reimport the pull request before landing.
@vassilmladenov has imported this pull request. If you are a Meta employee, you can view this in D79505213. (Because this pull request was imported automatically, there will not be any future comments.)
@vassilmladenov merged this pull request in facebook/hhvm@a455f079d5bc324514bc5740d0fafbd5aef5af9b.