Bug 71532 - "use strict" can not contain escape sequences or line continuation
Summary: "use strict" can not contain escape sequences or line continuation
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ariya Hidayat
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-11-03 18:56 PDT by Ariya Hidayat
Modified: 2011-11-07 18:25 PST (History)
0 users

See Also:


Attachments
patch (6.38 KB, patch)
2011-11-03 19:04 PDT, Ariya Hidayat
darin: review+
Details | Formatted Diff | Diff
updated patch (6.86 KB, patch)
2011-11-04 10:53 PDT, Ariya Hidayat
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ariya Hidayat 2011-11-03 18:56:39 PDT
"use strict" should not be recognized if it contains escape sequences or line continuation.

For example, the following should not trigger strict mode:

"use stri\
ct"
Comment 1 Ariya Hidayat 2011-11-03 19:04:50 PDT
Created attachment 113607 [details]
patch
Comment 2 Darin Adler 2011-11-04 10:18:09 PDT
Comment on attachment 113607 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=113607&action=review

> Source/JavaScriptCore/parser/Parser.cpp:144
> +                if (!hasSetStrict && directiveLiteralLength == 12 && m_globalData->propertyNames->useStrictIdentifier == *directive) {

This hardcoded 12 is not good. Can we put a named constant at the start of the file?

> Source/JavaScriptCore/parser/Parser.cpp:653
> +template <class TreeBuilder> TreeStatement Parser::parseStatement(TreeBuilder& context, const Identifier*& directive, unsigned *directiveLiteralLength)

misplaced * here
Comment 3 Ariya Hidayat 2011-11-04 10:37:50 PDT
> This hardcoded 12 is not good. Can we put a named constant at the start of the file?

What do you think of const in the function? It's closer and easier to discover.

> misplaced * here

Noted.

On the second thought, I also want to update to include escape sequence in the test (the patch only tests line continuation).
Comment 4 Ariya Hidayat 2011-11-04 10:53:17 PDT
Created attachment 113680 [details]
updated patch
Comment 5 Ariya Hidayat 2011-11-07 18:25:36 PST
Comment on attachment 113680 [details]
updated patch

Clearing flags on attachment: 113680

Committed r99513: <http://trac.webkit.org/changeset/99513>
Comment 6 Ariya Hidayat 2011-11-07 18:25:39 PST
All reviewed patches have been landed.  Closing bug.