Closed Bug 1454744 Opened 7 years ago Closed 7 years ago

[BinAST] Create proper bindings for positional formal parameters

Categories

(Core :: JavaScript Engine, enhancement, P2)

enhancement

Tracking

()

RESOLVED FIXED
mozilla61
Tracking Status
firefox61 --- fixed

People

(Reporter: efaust, Assigned: efaust)

Details

Attachments

(2 files)

No description provided.
This is modeled after (read: cargo culted) from the fields stuff. I tried commoning up the logic with some callbacks, but the helper got more complicated than the code duplication, so this is where I landed. I admit that in theory this isn't a great place to put parsing code, since it's just the structural induction step, but it's pretty convenient here.
Attachment #8968636 - Flags: review?(dteller)
Comment on attachment 8968636 [details] [diff] [review] Part 1: Augment BinSource parser generator to allow code in sum arms Review of attachment 8968636 [details] [diff] [review]: ----------------------------------------------------------------- lgtm ::: js/src/frontend/binsource/src/main.rs @@ +73,4 @@ > /// Custom per-field treatment. Used only for interfaces. > by_field: HashMap<FieldName, FieldRules>, > > + by_sum: HashMap<NodeName, SumRules>, Nit: Could you document this? @@ +775,4 @@ > (rules_for_this_node.build_result.is_some(), "build:"), > (rules_for_this_node.append.is_some(), "append:"), > (rules_for_this_node.by_field.len() > 0, "fields:"), > + (rules_for_this_node.by_sum.len() > 0, "sum-arms:"), Could you add this same verification to `fn generate_implement_interface`? Not urgent, that can wait for a followup bug.
Attachment #8968636 - Flags: review?(dteller) → review+
Comment on attachment 8968637 [details] [diff] [review] Part 2: add bindings for formal positional parameters Review of attachment 8968637 [details] [diff] [review]: ----------------------------------------------------------------- lgtm. However, sums are footgunish as the deanonymizer needs to transform `Foo = A|Bar, Bar = C|D` into `Foo = A|C|D, Bar = C|D`, so D ends up appearing in several places. So, can you double-check in the generated that you have applied it in all the places you meant to apply it? I have code in master to mitigate this problem, but I have not attempted to use it for binsource/src/main.rs. This might be needed for this use case.
Attachment #8968637 - Flags: review?(dteller) → review+
Priority: -- → P2
Pushed by efaustbmo@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/42c4d4acfba5 Part 1: Allow BinSource generator to place code after parsing interface sum branches. (r=Yoric) https://hg.mozilla.org/integration/mozilla-inbound/rev/02e7d188fd56 Part 2: Add bindings for formal parameter names. (r=Yoric)
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: