-
Notifications
You must be signed in to change notification settings - Fork 30
Use conformant API to iterate over environments #89
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
base: bugfix/env-iteration
Are you sure you want to change the base?
Conversation
|
Last commits also refactor construction of inspector node with a shared ctor. Introduction or formalisation of the notions of:
|
| #' | ||
| #' * "character" to show underlying entries in the global string pool. | ||
| #' * "environment" to show the underlying hashtables. | ||
| #' * "environment" to show binding components without any side effect (e.g. promise). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| #' * "environment" to show binding components without any side effect (e.g. promise). | |
| #' * "environment" to show binding components without any side effects (e.g. promises or active bindings). |
| "] ", | ||
| "<", | ||
| crayon::cyan(type), | ||
| length, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just checking, but no length in the placeholder branch either?
| case R_ENV_BINDING_TYPE_forced: | ||
| out.push_back(obj_addr_(r_env_binding_forced_value(x, sym))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it makes sense, but you've somewhat arbitrarily decided to pick the value over the expr for forced promises right?
| case EXTPTRSXP: return "EXTPTRSXP"; | ||
| case WEAKREFSXP: return "WEAKREFSXP"; | ||
| case RAWSXP: return "RAWSXP"; | ||
| case S4SXP: return "S4SXP"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm uncertain of whether we should call this S4SXP or the newer OBJSXP which was added for R7
https://github.com/wch/r-source/blob/e996f331169fa4b255dd720e6778fe937230f0a2/src/include/Rinternals.h#L137-L138
| forced [14] <PROMSXP> | ||
| _value [14] <REALSXP[1]> () | ||
| _code [15] <LANGSXP> () |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do 14 and 15 mean? Is this right? Are all 3 supposed to be 14?
Branched from #88.
Follows same approach as #88.
Unfortunately this means we are less precise in terms of object properties (references etc) because we can no longer get our hands on the intermediate nodes such as promises.