[webkit-reviews] review granted: [Bug 45289] Refactoring multiline comments in the lexer : [Attachment 66713] patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Sep 7 14:34:10 PDT 2010


Darin Adler <darin at apple.com> has granted Zoltan Herczeg
<zherczeg at webkit.org>'s request for review:
Bug 45289: Refactoring multiline comments in the lexer
https://bugs.webkit.org/show_bug.cgi?id=45289

Attachment 66713: patch
https://bugs.webkit.org/attachment.cgi?id=66713&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
> +	   if (UNLIKELY(m_current == '*')) {
> +	       shift();
> +	       if (m_current == '/') {
> +		   shift();
> +		   return true;
> +	       }
> +	       if (m_current == '*')
> +		   continue;
> +	   }

This ends up comparing with '*' twice, which seems like a shame given it's also
slightly confusing logic. Instead of the if statement and the continue
statement, we could just use while instead of if. Does that make things slower?


> @@ -1002,7 +1027,8 @@ inNumberAfterDecimalPoint:
>	   m_terminator = true;
>	   if (lastTokenWasRestrKeyword()) {
>	       token = SEMICOLON;
> -	       goto doneSemicolon;
> +	       m_delimited = true;
> +	       goto returnToken;
>	   }
>	   goto start;
>      case CharacterInvalid:

Why is this change included in the same patch? It seems unrelated and should be
done based on its own merits, not as a ridealong. We can keep the doneSemicolon
symbol.


More information about the webkit-reviews mailing list