Skip to content
This repository has been archived by the owner on Feb 12, 2022. It is now read-only.

Fix CSE in back-end #2516

Closed
wants to merge 1 commit into from
Closed

Fix CSE in back-end #2516

wants to merge 1 commit into from

Conversation

hermanventer
Copy link
Contributor

Release note: none

This fixes two problems uncovered while debugging the failure of PR #2460 when run on a large internal test case:

  1. CSE did not fix up recently introduced aliases of expressions.
  2. The serializer logic visited array elements that were not visited because of recently introduced leak logic.

Copy link

@sb98052 sb98052 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. This fixes a very gnarly problem that has also been blocking me. On the the leaking logic - I see the issue - we can only omit the serialization of properties as literals if we know them to have been materialized. This does not include the index properties of an array. I will investigate this further and follow up.

Copy link

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hermanventer is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@hermanventer hermanventer deleted the serializerFixes branch August 31, 2018 18:31
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants