Closed Bug 1437955 Opened 5 years ago Closed 5 years ago

Remove ParseNodeKind::Semi and Nop

Categories

(Core :: JavaScript Engine, enhancement, P3)

Product:
Core
Component:
JavaScript Engine
Type:
enhancement

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla60
Tracking Flags:
Tracking Status
firefox60 --- fixed

People

(Reporter: jorendorff, Assigned: jorendorff)

Details

Attachments

(3 files)

Part 1: Split ParseNodeKind::Semi into ExpressionStatement and EmptyStatement
26.50 KB, patch
Waldo
: review+
Details | Diff | Splinter Review
Part 2: Remove unused ParseNodeKind::Nop
6.43 KB, patch
Waldo
: review+
Details | Diff | Splinter Review
Part 0: Fix places where bug 1429206 left old filenames in comments
8.63 KB, patch
jandem
: review+
Details | Diff | Splinter Review 
"What kind of parse node is it?"

"It's a nop"

TypeError
Assignee: nobody → jorendorff
Status: NEW → ASSIGNED

        Attachment #8950657 -
        Flags: review?(jwalden+bmo)

    
  
Summary: kill ParseNodeKind::Semi and Nop → Remove ParseNodeKind::Semi and Nop

    
  

          status-firefox60:
          --- → fix-optional
Priority: -- → P3

    
    

    
  
Comment on attachment 8950656 [details] [diff] [review]
Part 1: Split ParseNodeKind::Semi into ExpressionStatement and EmptyStatement

Review of attachment 8950656 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/wasm/AsmJS.cpp
@@ +7160,5 @@
> +      case ParseNodeKind::Return:               return CheckReturn(f, stmt);
> +      case ParseNodeKind::StatementList:        return CheckStatementList(f, stmt);
> +      case ParseNodeKind::Break:                return CheckBreakOrContinue(f, true, stmt);
> +      case ParseNodeKind::Continue:             return CheckBreakOrContinue(f, false, stmt);
> +      case ParseNodeKind::LexicalScope:         return CheckLexicalScope(f, stmt);

If you're touching all these lines, could I possibly trouble you to undo the single-lineness here?  Case on one line, subsequent return statement on the next, blank line between implied case-blocks.  Horizontal alignment of things after other things is just dumb and not worth the trouble.

        Attachment #8950656 -
        Flags: review?(jwalden+bmo) → review+

        Attachment #8950657 -
        Flags: review?(jwalden+bmo) → review+

    
    

    
  
Comment on attachment 8951647 [details] [diff] [review]
Part 0: Fix places where bug 1429206 left old filenames in comments

Review of attachment 8951647 [details] [diff] [review]:
-----------------------------------------------------------------

Good catch.

        Attachment #8951647 -
        Flags: review?(jdemooij) → review+

    
    

    
  
Oh no, that patch is in the wrong bug. Derp.

    
    

    
  
https://hg.mozilla.org/integration/mozilla-inbound/rev/e21126825c41be2a70bb6759e2dbdc7190621608
Bug 1437955 - Part 1: Split ParseNodeKind::Semi into ExpressionStatement and EmptyStatement. r=Waldo.

https://hg.mozilla.org/integration/mozilla-inbound/rev/b593a217bb5a4bd160dff15f1fc1577b95f06778
Bug 1437955 - Part 2: Remove unused ParseNodeKind::Nop. r=Waldo.

    
    

    
  
(The right bug for "Part 0" is bug 1438278.)

    
    

    
  
https://hg.mozilla.org/mozilla-central/rev/e21126825c41
https://hg.mozilla.org/mozilla-central/rev/b593a217bb5a
Status: ASSIGNED → RESOLVED
Closed: 5 years ago

          status-firefox60:
          fix-optional → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla60

          You need to log in
          before you can comment on or make changes to this bug.