make constant-folding in the parser optional

RESOLVED FIXED

Status

()

Core
JavaScript Engine
RESOLVED FIXED
7 years ago
5 years ago

People

(Reporter: dherman, Assigned: dherman)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: reflect-parse fixed-in-tracemonkey)

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

7 years ago
It should be possible to turn off js_FoldConstants and FoldXMLConstants for the parser API, so users can get an AST that's closer to the actual input. This could easily be turned off with a boolean flag (which might not affect perf noticeably) or if necessary, templatization.

Dave
(In reply to comment #0)
> could easily be turned off with a boolean flag (which might not affect perf
> noticeably) or if necessary, templatization.

Definitely not going to affect perf if you're guarding a whole tree traversal with a single highly-predictable branch. (Correct me if I haven't understood correctly.)
Emitter expects constant folder to have run, IIRC. So this would be for AST output only, at least without other changes.

/be
(Assignee)

Comment 3

7 years ago
> Definitely not going to affect perf if you're guarding a whole tree traversal
> with a single highly-predictable branch. (Correct me if I haven't understood
> correctly.)

IIUC, the parser does recursively call the constant folder in the TOK_DELETE case, but I'd expect this to be rare enough not to matter.

Dave
(Assignee)

Comment 4

7 years ago
> Emitter expects constant folder to have run, IIRC. So this would be for AST
> output only, at least without other changes.

Right.

Dave
(Assignee)

Updated

7 years ago
Whiteboard: reflect-parse
(Assignee)

Updated

7 years ago
Blocks: 632056
(Assignee)

Updated

7 years ago
Blocks: 632029
(Assignee)

Updated

7 years ago
Blocks: 632026
(Assignee)

Comment 5

7 years ago
Created attachment 511556 [details] [diff] [review]
template-izes js::Parser to make constant-folding optional

Attaching an experimental patch that makes it possible to disable the use of js_FoldConstants. It leaves constant-folding on normally since (as mentioned in comment 2) it's required for the emitter. But it disables it for Reflect.parse.

The parser still performs constant-folding for very simple arithmetic expressions.

Dave
(Assignee)

Comment 6

6 years ago
Created attachment 537743 [details] [diff] [review]
dynamic flag, no template nonsense

This patch is *vastly* simpler, using just a dynamic flag and no template silliness. It's pushed on top of a couple other pending patches in my hg queue so may not apply without failures. I'll try to land those other patches ASAP.

Dave
Attachment #511556 - Attachment is obsolete: true
Comment on attachment 537743 [details] [diff] [review]
dynamic flag, no template nonsense

This wasn't r?anyone, but I'm for it! It needs a test, though. r=me with that.
Attachment #537743 - Flags: review+
(Assignee)

Comment 8

6 years ago
http://hg.mozilla.org/tracemonkey/rev/9b37d1f464e5
Whiteboard: reflect-parse → reflect-parse fixed-in-tracemonkey
cdleary-bot mozilla-central merge info:
http://hg.mozilla.org/mozilla-central/rev/9b37d1f464e5
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
(Assignee)

Updated

5 years ago
Duplicate of this bug: 722087
(Assignee)

Updated

5 years ago
Duplicate of this bug: 659636
You need to log in before you can comment on or make changes to this bug.