Skip to content

Deprecate the USES_DYNAMIC_ALLOC setting #19164

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 12, 2023
Merged

Conversation

sbc100
Copy link
Collaborator

@sbc100 sbc100 commented Apr 12, 2023

All this setting was doing was setting MALLOC=none so it doesn't seem like it needs to exist as a separate setting.

I think it used to do more, for example, prior to #12790 it used was used in some JS library code. Another PR that removed a lot of usage of this settings was #12057.

Simply removing it simplifies another change I'm working on: #18905.

Also, the docs were incorrect in stating it was ignored except under MINIMAL_RUNTIME, but we had no testing for it.

@sbc100 sbc100 requested review from kripken, juj and dschuff April 12, 2023 17:27
Copy link
Member

@kripken kripken left a comment

Choose a reason for hiding this comment

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

Please mention this in the changelog.

All this setting was doing was setting MALLOC=none so it doesn't seem
like it needs to exist as a separate setting.

I think it used to do more, for example, prior to #12790 it used was
used in some JS library code.  Another PR that removed a lot of usage
of this settings was #12057.

Simply removing it simplifies another change I'm working on: #18905.

Also, the docs were incorrect in stating it was ignored except under
MINIMAL_RUNTIME, but we had no testing for it.
@sbc100 sbc100 force-pushed the deprectate_USES_DYNAMIC_ALLOC branch from 437c7b3 to 3e635e8 Compare April 12, 2023 21:08
@sbc100 sbc100 merged commit e64f644 into main Apr 12, 2023
@sbc100 sbc100 deleted the deprectate_USES_DYNAMIC_ALLOC branch April 12, 2023 21:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants