Skip to content

Conversation

@akosthekiss
Copy link
Member

Also remove the legacy jerry_get_memory_limits API function as it
does not report either useful or valid information.

JerryScript-DCO-1.0-Signed-off-by: Akos Kiss akiss@inf.u-szeged.hu

@akosthekiss akosthekiss added the api Related to the public API label May 14, 2018
@akosthekiss
Copy link
Member Author

NOTE: THIS IS AN API-BREAKING CHANGE! (At least, theoretically.)

In practice, I cannot think of any user of JerryScript who would have used the jerry_get_memory_limits function. It has been introduced in Feb 11, 2015 (35fa39c) but hasn't been of any use since Feb 13, 2015 (43ea53b), when its counterpart jrt_set_mem_limits was macrod out to never get enabled ever again.

@akosthekiss
Copy link
Member Author

Speaking of config macros and breaking changes: is the CONFIG_ECMA_GLOBAL_ENVIRONMENT_DECLARATIVE setting used / useful? The only place where this is used at is the following. Which doesn't seem to be too standard. It's also been introduced several years ago (Sep 2014 (fbcd393)) but I'm not aware of any (current?) use cases.

void
ecma_init_global_lex_env (void)
{
#ifdef CONFIG_ECMA_GLOBAL_ENVIRONMENT_DECLARATIVE
  JERRY_CONTEXT (ecma_global_lex_env_p) = ecma_create_decl_lex_env (NULL);
#else /* !CONFIG_ECMA_GLOBAL_ENVIRONMENT_DECLARATIVE */
  ecma_object_t *glob_obj_p = ecma_builtin_get (ECMA_BUILTIN_ID_GLOBAL);

  JERRY_CONTEXT (ecma_global_lex_env_p) = ecma_create_object_lex_env (NULL, glob_obj_p, false);

  ecma_deref_object (glob_obj_p);
#endif /* CONFIG_ECMA_GLOBAL_ENVIRONMENT_DECLARATIVE */
} /* ecma_init_global_lex_env */

I'm happy to include the removal of the macro and the associated code paths in this patch but feedback would be appreciated on this first.

@zherczeg
Copy link
Member

#2213

@LaszloLango LaszloLango added this to the Release 2.0 milestone May 15, 2018
@akosthekiss akosthekiss force-pushed the remove-unused-config branch from 3e9d3d5 to bbf2608 Compare May 16, 2018 14:41
@zherczeg
Copy link
Member

I would emphasize the removal of jerry_get_memory_limits rather than macros in the title and commit log. Other than that is ok for me.

@akosthekiss
Copy link
Member Author

@zherczeg And what about the question raised in the comment: what's the deal with CONFIG_ECMA_GLOBAL_ENVIRONMENT_DECLARATIVE ?

@zherczeg
Copy link
Member

That macro makes an ecmascript program behave like a function without the argument and return support. It is non-standard.

Currently you can create functions directly, and I would recommend that if you want to use such a feature.

Copy link
Contributor

@LaszloLango LaszloLango left a comment

Choose a reason for hiding this comment

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

LGTM

@akosthekiss akosthekiss force-pushed the remove-unused-config branch from bbf2608 to 33a8e2e Compare May 17, 2018 09:52
@akosthekiss akosthekiss changed the title Remove unused configuration macros Remove legacy jerry_get_memory_limits API function and unused configuration macros May 17, 2018
@akosthekiss
Copy link
Member Author

I've updated the PR according to the feedbacks:

  • PR title and commit message changed
  • removed CONFIG_ECMA_GLOBAL_ENVIRONMENT_DECLARATIVE from the code base
  • rebased onto latest master

Copy link
Member

@zherczeg zherczeg left a comment

Choose a reason for hiding this comment

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

LGTM

…ration macros

JerryScript-DCO-1.0-Signed-off-by: Akos Kiss akiss@inf.u-szeged.hu
@akosthekiss akosthekiss force-pushed the remove-unused-config branch from 33a8e2e to 2f6ee92 Compare May 17, 2018 13:11
@akosthekiss akosthekiss merged commit 13bd30f into jerryscript-project:master May 18, 2018
@akosthekiss akosthekiss deleted the remove-unused-config branch May 18, 2018 08:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api Related to the public API

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants