1184597 - Unnecessary test and assignment in Parser<FullParseHandler>::forStatement

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[André Bargull [:anba]]

Just happened to notice this, could be cleaned up when someone touches that method (or the parser in general) again.


http://hg.mozilla.org/mozilla-central/file/d8023b434e25/js/src/frontend/Parser.cpp#l4927
---
if (headKind == PNK_FOROF) {
    forStmt.type = (headKind == PNK_FOROF) ? STMT_FOR_OF_LOOP : STMT_FOR_IN_LOOP;
    ...
}
---

No reason to check |headKind| twice.




http://hg.mozilla.org/mozilla-central/file/d8023b434e25/js/src/frontend/Parser.cpp#l5020
---
headKind = PNK_FORHEAD;
---

The assignment to |headKind| is not needed, cf. 
http://hg.mozilla.org/mozilla-central/file/d8023b434e25/js/src/frontend/Parser.cpp#l4907
http://hg.mozilla.org/mozilla-central/file/d8023b434e25/js/src/frontend/Parser.cpp#l4918

Comment 1

[Benjamin Bouvier [:bbouvier] (inactive)]

Attachment: redundant.patch

Wow, nice find (even Coverity hasn't found it!). A quite simple patch too.

Comment 2

[André Bargull [:anba]]

(In reply to Benjamin Bouvier [:bbouvier] from comment #1)
> Created attachment 8635223 [details] [diff] [review]
> redundant.patch
> 
> Wow, nice find (even Coverity hasn't found it!). A quite simple patch too.

To tell the truth I was unsure about filing this issue because it's that simple. :)

The reassignment to headKind in l.5020 can be removed, too. Right?

Comment 3

[Eric Faust [:efaust]]

Comment on attachment 8635223 [details] [diff] [review]
redundant.patch

uh. Yeahp.

Comment 4

[Benjamin Bouvier [:bbouvier] (inactive)]

(In reply to André Bargull from comment #2)
> (In reply to Benjamin Bouvier [:bbouvier] from comment #1)
> > Created attachment 8635223 [details] [diff] [review]
> > redundant.patch
> > 
> > Wow, nice find (even Coverity hasn't found it!). A quite simple patch too.
> 
> To tell the truth I was unsure about filing this issue because it's that
> simple. :)
> 
> The reassignment to headKind in l.5020 can be removed, too. Right?

Indeed, I've changed it to an assert.

Comment 5

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2ba437578597

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/2ba437578597