Skip to content
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

util.inspect not checking for loops in {,Weak}Map data #14758

Closed
rdeforest opened this issue Aug 11, 2017 · 1 comment
Closed

util.inspect not checking for loops in {,Weak}Map data #14758

rdeforest opened this issue Aug 11, 2017 · 1 comment
Labels
confirmed-bug Issues with confirmed bugs. util Issues and PRs related to the built-in util module.

Comments

@rdeforest
Copy link

  • Version: v8.2.1
  • Platform: Darwin a0999b0cf96f.ant.amazon.com 16.6.0 Darwin Kernel Version 16.6.0: Fri Apr 14 16:21:16 PDT 2017; root:xnu-3789.60.24~6/RELEASE_X86_64 x86_64
  • Subsystem: util

Expected behavior:

coffee> m = new Map
Map {}
coffee> m.set m, m
ref1 = Map {
  ref1 => ref1
}
coffee>

Actual behavior:

coffee> m = new Map
Map {}
coffee> m.set m, m
Map {
  Map {
  Map { [Object] => [Object] } => Map { [Object] => [Object] } } => Map {
  Map { [Object] => [Object] } => Map { [Object] => [Object] } } }
coffee>

I know this is a non-trivial problem to solve but it seems worth at least documenting if not solving.

@rdeforest rdeforest changed the title util.format not checking for loops in {,Weak}Map data util.inspect not checking for loops in {,Weak}Map data Aug 11, 2017
@mscdex mscdex added the util Issues and PRs related to the built-in util module. label Aug 11, 2017
@TimothyGu TimothyGu added the confirmed-bug Issues with confirmed bugs. label Aug 11, 2017
@TimothyGu
Copy link
Member

Can reproduce on Node.js' REPL:

> var m = new Map()
undefined
> m.set('m', m)
Map { 'm' => Map { 'm' => Map { 'm' => [Object] } } }

Note, this works fine with a normal object:

> var o = {}
undefined
> o.o = o
{ o: [Circular] }
> o
{ o: [Circular] }

@aqrln aqrln self-assigned this Aug 11, 2017
BridgeAR added a commit to BridgeAR/node that referenced this issue Aug 11, 2017
aqrln added a commit to aqrln/node that referenced this issue Aug 11, 2017
Handle maps and sets with circular references in `util.inspect()` the
way objects and arrays are treated in this case.

Fixes: nodejs#14758
@aqrln aqrln removed their assignment Aug 11, 2017
addaleax added a commit that referenced this issue Aug 17, 2017
Circular references are conceptually nothing that should be checked
for objects (or Sets or Maps) only, but applies to recursive structures
in general, so move the `seen` checks into a position where it is part
of the recursion itself.

Fixes: #14758
PR-URL: #14790
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben.bridgewater@fintura.de>
MylesBorins pushed a commit that referenced this issue Sep 10, 2017
Circular references are conceptually nothing that should be checked
for objects (or Sets or Maps) only, but applies to recursive structures
in general, so move the `seen` checks into a position where it is part
of the recursion itself.

Fixes: #14758
PR-URL: #14790
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben.bridgewater@fintura.de>
MylesBorins pushed a commit that referenced this issue Sep 12, 2017
Circular references are conceptually nothing that should be checked
for objects (or Sets or Maps) only, but applies to recursive structures
in general, so move the `seen` checks into a position where it is part
of the recursion itself.

Fixes: #14758
PR-URL: #14790
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben.bridgewater@fintura.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed-bug Issues with confirmed bugs. util Issues and PRs related to the built-in util module.
Projects
None yet
4 participants