Open Bug 1650188 Opened 1 year ago Updated 2 months ago

[META] Private Fields Support for Devtools

Categories

(DevTools :: Console, task)

task

Tracking

(Not tracked)

People

(Reporter: mgaudet, Unassigned)

References

(Blocks 1 open bug)

Details

(Keywords: meta)

Attachments

(2 files)

Attached image Firefox Devtools Issues

Right now the Private Fields JS language features is under development under the private-fields bug tree.

I did some testing, and as I had suspected, there will need to be some cooperation between DevTools and the JS engine to make private fields work well. I'll need some guidance help provide the right interfaces.

Attached is a screenshot showing at least two different issues I found while running the build I created on try atop the not-yet-landed patches in Bug 1642476.

  1. The first issue is that we don't show private fields in devtools. This is not unexpected: It is intended that from a language-specification perspective that private fields are not observable via any reflection system; this means that we'll have to provide a side-entry to devtools to bypass this.
  2. The act of adding a private field to an object is not idempotent; it appears that while I was experimenting with the peculiar nature of private fields, the "show the result of the computation" feature stamped window for me, then when I hit enter I got an exception, as you can't add private fields twice.
Attached image Chrome Devtools.png

The Chrome Devtools screenshot is attached to contrast with what currently exists for Firefox.

Thanks Matthew for filing this bug.
For the second error, I think we need to exclude instant evaluation when the class has private fields.
We might want to sync up with Logan to know what our options are

Type: enhancement → task
Component: General → Console
Flags: needinfo?(loganfsmyth)

The second issue is, I would guess, because https://hg.mozilla.org/mozilla-central/diff/937d4dcd886732318b8722ee66052463a6c4bf19/js/src/debugger/Script.cpp put JSOp::InitPrivateElem in the section for sideeffect-free bytecodes. I assume that was following the lead because the other Init* bytecodes are in there, but at least for this one, this means you can initialize an element on an object that already existed. AFAIK the assumption for the other Init* bytecodes is that if you're initializing a property for the first time, the object that the property is attached to is also newly-allocated. That assumption does not hold up in for private fields in this case, so we'll need to move that to be an effectful bytecode.

Do you happen to know if any of the other Init* bytecodes will have similar issues with class fields?

Flags: needinfo?(loganfsmyth)

For the first point, I could imagine a debuggerObject.getOwnPrivateNames() to go along with getOwnPropertyNames and getOwnPropertySymbols, but I'm not sure exactly what we'd want to return. I see that internally the names are symbols, I'm assuming we'd want the debugger to treat that as an implementation detail, right? So we'd need a new Debugger.PrivateName object to wrap the symbol, and then we can use the normal debuggerObject.getProperty() to get the value itself.

(In reply to Logan Smyth [:loganfsmyth] from comment #3)

The second issue is, I would guess, because https://hg.mozilla.org/mozilla-central/diff/937d4dcd886732318b8722ee66052463a6c4bf19/js/src/debugger/Script.cpp put JSOp::InitPrivateElem in the section for sideeffect-free bytecodes. I assume that was following the lead because the other Init* bytecodes are in there, but at least for this one, this means you can initialize an element on an object that already existed. AFAIK the assumption for the other Init* bytecodes is that if you're initializing a property for the first time, the object that the property is attached to is also newly-allocated. That assumption does not hold up in for private fields in this case, so we'll need to move that to be an effectful bytecode.

Do you happen to know if any of the other Init* bytecodes will have similar issues with class fields?

Ah -- that's a mea-culpa then. I'll prep a patch to fix that and open a separate bug. Regarding your Q about fields: We're definitely in a bit of trouble; I'll open a new bug and cc you on it. I'm not sure what the right fix is there; it may be we cannot classify any of the initelem bytecodes as effecting.

Depends on: 1651420

(In reply to Logan Smyth [:loganfsmyth] from comment #4)

For the first point, I could imagine a debuggerObject.getOwnPrivateNames() to go along with getOwnPropertyNames and getOwnPropertySymbols, but I'm not sure exactly what we'd want to return. I see that internally the names are symbols, I'm assuming we'd want the debugger to treat that as an implementation detail, right? So we'd need a new Debugger.PrivateName object to wrap the symbol, and then we can use the normal debuggerObject.getProperty() to get the value itself.

Yeah, we definitely don't want to expose the name as a symbol.

See Also: → 1499679

let's turn this bug into a meta

Keywords: meta
Summary: Private Fields Support for Devtools → [META] Private Fields Support for Devtools
Depends on: 1709538
Depends on: 1709542
Depends on: 1709544
Depends on: 1709567
Depends on: 1709800
Depends on: 1709956
Depends on: 1710417
See Also: 1499679
Depends on: 1499679
You need to log in before you can comment on or make changes to this bug.