101013 2012-11-01 22:02:30 -0700 Lexer::scanRegExp, create 8 bit pattern and flag Identifiers from 16 bit source when possible 2022-02-27 23:26:14 -0800 1 1 1 Unclassified WebKit JavaScriptCore 528+ (Nightly build) All All RESOLVED FIXED P2 Normal --- 1 msaboff msaboff oldest_to_newest 756884 0 msaboff 2012-11-01 22:02:30 -0700 scanRegExp() currently creates Identifiers of the same bitness as the source. Many regular expressions in 16 bit sources consist of only 8 bit characters. 757580 1 172135 msaboff 2012-11-02 14:25:39 -0700 Created attachment 172135 Patch 759776 2 172135 darin 2012-11-06 09:34:58 -0800 Comment on attachment 172135 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=172135&action=review > Source/JavaScriptCore/parser/Lexer.cpp:1689 > template <typename T> > +static inline void orCharacter(UChar&, UChar); I don’t think the static here has any effect anything. > Source/JavaScriptCore/parser/Lexer.cpp:1692 > +template <> > +inline void orCharacter<LChar>(UChar&, UChar) { } Don’t we want static here? > Source/JavaScriptCore/parser/Lexer.cpp:1695 > +template <> > +inline void orCharacter<UChar>(UChar& orAccumulator, UChar character) Don’t we want static here? > Source/JavaScriptCore/parser/Lexer.cpp:1707 > + UChar orAllChars = 0; It’s awkward that “or all chars” is a verb phrase. I’d suggest a noun phrase, "charactersOredTogether" or something like that. 759997 3 msaboff 2012-11-06 13:28:48 -0800 (In reply to comment #2) > (From update of attachment 172135 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=172135&action=review > > > Source/JavaScriptCore/parser/Lexer.cpp:1689 > > template <typename T> > > +static inline void orCharacter(UChar&, UChar); > > I don’t think the static here has any effect anything. I think it provide the same file scoping as it would to a normal function declaration. I'll verify by looking at the exported symbols from the .o file. > > Source/JavaScriptCore/parser/Lexer.cpp:1692 > > +template <> > > +inline void orCharacter<LChar>(UChar&, UChar) { } > > Don’t we want static here? No, the static can't be with the explicit specialization. The compiler flags an error. > > Source/JavaScriptCore/parser/Lexer.cpp:1695 > > +template <> > > +inline void orCharacter<UChar>(UChar& orAccumulator, UChar character) > > Don’t we want static here? No. Ditto. > > Source/JavaScriptCore/parser/Lexer.cpp:1707 > > + UChar orAllChars = 0; > > It’s awkward that “or all chars” is a verb phrase. I’d suggest a noun phrase, "charactersOredTogether" or something like that. Will do. 760013 4 msaboff 2012-11-06 13:48:45 -0800 I tried the template declaration with and without the "static". With the static, the symbols for the two instance are local, but without the static they are global. 760026 5 msaboff 2012-11-06 14:02:23 -0800 Committed r133668: <http://trac.webkit.org/changeset/133668> 172135 2012-11-02 14:25:39 -0700 2022-02-27 23:26:14 -0800 Patch 101013.patch text/plain 5179 msaboff SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gMTMzMzQxKQorKysgU291cmNl L0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDI1IEBA CisyMDEyLTExLTAyICBNaWNoYWVsIFNhYm9mZiAgPG1zYWJvZmZAYXBwbGUuY29tPgorCisgICAg ICAgIExleGVyOjpzY2FuUmVnRXhwLCBjcmVhdGUgOCBiaXQgcGF0dGVybiBhbmQgZmxhZyBJZGVu dGlmaWVycyBmcm9tIDE2IGJpdCBzb3VyY2Ugd2hlbiBwb3NzaWJsZQorICAgICAgICBodHRwczov L2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTAxMDEzCisKKyAgICAgICAgUmV2aWV3 ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgQ2hhbmdlZCBzY2FuUmVnRXhwIHNvIHRo YXQgaXQgd2lsbCBjcmVhdGUgOCBiaXQgaWRlbnRpZmllcnMgZnJvbSA4IGJpdCBzb3VyY2VzIGFu ZCBmcm9tIDE2IGJpdCBzb3VyY2VzCisgICAgICAgIHdoYW4gYWxsIHRoZSBjaGFyYWN0ZXJzIGFy ZSA4IGJpdC4gIFVzaW5nIHR3byB0ZW1wbGF0ZWQgaGVscGVycywgdGhlICJpcyBhbGwgOCBiaXQi IGNoZWNrIGlzIG9ubHkgcGVyZm9ybWVkCisgICAgICAgIG9uIDE2IGJpdCBzb3VyY2VzLiAgVGhl IGZpcnN0IGhlbHBlciBpcyBvckNoYXJhY3RlcigpIHRoYXQgd2lsbCBhY2N1bXVsYXRlIHRoZSBv ciB2YWx1ZSBvZiBhbGwgY2hhcmFjdGVycworICAgICAgICBvbmx5IGZvciAxNiBiaXQgc291cmNl cy4gIFJlcGxhY2VkIHRoZSBoZWxwZXIgTGV4ZXI6Om1ha2VJZGVudGlmaWVyU2FtZVR5cGUoKSB3 aXRoIExleGVyOjptYWtlUmlnaHRTaXplZElkZW50aWZpZXIoKS4KKworICAgICAgICAqIHBhcnNl ci9MZXhlci5jcHA6CisgICAgICAgIChKU0M6Om9yQ2hhcmFjdGVyPExDaGFyPik6IEV4cGxpY2l0 IHRlbXBsYXRlIHRoYXQgc2VydmVzIGFzIGEgcGxhY2Vob2xkZXIuCisgICAgICAgIChKU0M6Om9y Q2hhcmFjdGVyPFVDaGFyPik6IEV4cGxpY2l0IHRlbXBsYXRlIHRoYXQgYWN0dWFsbHkgb3IgYWNj dW11bGF0ZXMgY2hhcmFjdGVycy4KKyAgICAgICAgKEpTQzo6TGV4ZXI6OnNjYW5SZWdFeHApOgor ICAgICAgICAqIHBhcnNlci9MZXhlci5oOgorICAgICAgICAoTGV4ZXIpOgorICAgICAgICAoSlND OjpMZXhlcjo6bWFrZVJpZ2h0U2l6ZWRJZGVudGlmaWVyPExDaGFyPik6IE5ldyB0ZW1wbGF0ZSB0 aGF0IGFsd2F5cyBjcmVhdGVzIGFuIDggYml0IElkZW50aWZpZXIuCisgICAgICAgIChKU0M6Okxl eGVyOjptYWtlUmlnaHRTaXplZElkZW50aWZpZXI8VUNoYXI+KTogTmV3IHRlbXBsYXRlIHRoYXQg Y3JlYXRlcyBhbiA4IGJpdCBJZGVudGlmaWVyIGZvciA4IGJpdAorICAgICAgICBkYXRhIGluIGEg MTYgYml0IHNvdXJjZS4KKwogMjAxMi0xMS0wMiAgTWljaGFlbCBTYWJvZmYgIDxtc2Fib2ZmQGFw cGxlLmNvbT4KIAogICAgICAgICBSZWdFeHAucHJvdG90eXBlLnRvU3RyaW5nIFNob3VsZCBQcm9k dWNlIGFuIDggYml0IEpTU3RyaW5nIGlmIHBvc3NpYmxlLgpJbmRleDogU291cmNlL0phdmFTY3Jp cHRDb3JlL3BhcnNlci9MZXhlci5jcHAKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNlL0phdmFTY3JpcHRD b3JlL3BhcnNlci9MZXhlci5jcHAJKHJldmlzaW9uIDEzMjk3MCkKKysrIFNvdXJjZS9KYXZhU2Ny aXB0Q29yZS9wYXJzZXIvTGV4ZXIuY3BwCSh3b3JraW5nIGNvcHkpCkBAIC0xNjg2LDEyICsxNjg2 LDI1IEBAIHJldHVybkVycm9yOgogfQogCiB0ZW1wbGF0ZSA8dHlwZW5hbWUgVD4KK3N0YXRpYyBp bmxpbmUgdm9pZCBvckNoYXJhY3RlcihVQ2hhciYsIFVDaGFyKTsKKwordGVtcGxhdGUgPD4KK2lu bGluZSB2b2lkIG9yQ2hhcmFjdGVyPExDaGFyPihVQ2hhciYsIFVDaGFyKSB7IH0KKwordGVtcGxh dGUgPD4KK2lubGluZSB2b2lkIG9yQ2hhcmFjdGVyPFVDaGFyPihVQ2hhciYgb3JBY2N1bXVsYXRv ciwgVUNoYXIgY2hhcmFjdGVyKQoreworICAgIG9yQWNjdW11bGF0b3IgfD0gY2hhcmFjdGVyOwor fQorCit0ZW1wbGF0ZSA8dHlwZW5hbWUgVD4KIGJvb2wgTGV4ZXI8VD46OnNjYW5SZWdFeHAoY29u c3QgSWRlbnRpZmllciomIHBhdHRlcm4sIGNvbnN0IElkZW50aWZpZXIqJiBmbGFncywgVUNoYXIg cGF0dGVyblByZWZpeCkKIHsKICAgICBBU1NFUlQobV9idWZmZXIxNi5pc0VtcHR5KCkpOwogCiAg ICAgYm9vbCBsYXN0V2FzRXNjYXBlID0gZmFsc2U7CiAgICAgYm9vbCBpbkJyYWNrZXRzID0gZmFs c2U7CisgICAgVUNoYXIgb3JBbGxDaGFycyA9IDA7CiAKICAgICBpZiAocGF0dGVyblByZWZpeCkg ewogICAgICAgICBBU1NFUlQoIWlzTGluZVRlcm1pbmF0b3IocGF0dGVyblByZWZpeCkpOwpAQCAt MTcxNCw2ICsxNzI3LDcgQEAgYm9vbCBMZXhlcjxUPjo6c2NhblJlZ0V4cChjb25zdCBJZGVudGlm aQogICAgICAgICAgICAgYnJlYWs7CiAKICAgICAgICAgcmVjb3JkMTYocHJldik7CisgICAgICAg IG9yQ2hhcmFjdGVyPFQ+KG9yQWxsQ2hhcnMsIHByZXYpOwogCiAgICAgICAgIGlmIChsYXN0V2Fz RXNjYXBlKSB7CiAgICAgICAgICAgICBsYXN0V2FzRXNjYXBlID0gZmFsc2U7CkBAIC0xNzMzLDE1 ICsxNzQ3LDE4IEBAIGJvb2wgTGV4ZXI8VD46OnNjYW5SZWdFeHAoY29uc3QgSWRlbnRpZmkKICAg ICAgICAgfQogICAgIH0KIAotICAgIHBhdHRlcm4gPSBtYWtlSWRlbnRpZmllclNhbWVUeXBlKG1f YnVmZmVyMTYuZGF0YSgpLCBtX2J1ZmZlcjE2LnNpemUoKSk7CisgICAgcGF0dGVybiA9IG1ha2VS aWdodFNpemVkSWRlbnRpZmllcihtX2J1ZmZlcjE2LmRhdGEoKSwgbV9idWZmZXIxNi5zaXplKCks IG9yQWxsQ2hhcnMpOworCiAgICAgbV9idWZmZXIxNi5yZXNpemUoMCk7CisgICAgb3JBbGxDaGFy cyA9IDA7CiAKICAgICB3aGlsZSAoaXNJZGVudFBhcnQobV9jdXJyZW50KSkgewogICAgICAgICBy ZWNvcmQxNihtX2N1cnJlbnQpOworICAgICAgICBvckNoYXJhY3RlcjxUPihvckFsbENoYXJzLCBt X2N1cnJlbnQpOwogICAgICAgICBzaGlmdCgpOwogICAgIH0KIAotICAgIGZsYWdzID0gbWFrZUlk ZW50aWZpZXJTYW1lVHlwZShtX2J1ZmZlcjE2LmRhdGEoKSwgbV9idWZmZXIxNi5zaXplKCkpOwor ICAgIGZsYWdzID0gbWFrZVJpZ2h0U2l6ZWRJZGVudGlmaWVyKG1fYnVmZmVyMTYuZGF0YSgpLCBt X2J1ZmZlcjE2LnNpemUoKSwgb3JBbGxDaGFycyk7CiAgICAgbV9idWZmZXIxNi5yZXNpemUoMCk7 CiAKICAgICByZXR1cm4gdHJ1ZTsKSW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9wYXJzZXIv TGV4ZXIuaAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvSmF2YVNjcmlwdENvcmUvcGFyc2VyL0xleGVy LmgJKHJldmlzaW9uIDEzMjk3MCkKKysrIFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9wYXJzZXIvTGV4 ZXIuaAkod29ya2luZyBjb3B5KQpAQCAtMTQ4LDcgKzE0OCw3IEBAIHByaXZhdGU6CiAgICAgQUxX QVlTX0lOTElORSBjb25zdCBJZGVudGlmaWVyKiBtYWtlSWRlbnRpZmllcihjb25zdCBVQ2hhciog Y2hhcmFjdGVycywgc2l6ZV90IGxlbmd0aCk7CiAgICAgQUxXQVlTX0lOTElORSBjb25zdCBJZGVu dGlmaWVyKiBtYWtlTENoYXJJZGVudGlmaWVyKGNvbnN0IExDaGFyKiBjaGFyYWN0ZXJzLCBzaXpl X3QgbGVuZ3RoKTsKICAgICBBTFdBWVNfSU5MSU5FIGNvbnN0IElkZW50aWZpZXIqIG1ha2VMQ2hh cklkZW50aWZpZXIoY29uc3QgVUNoYXIqIGNoYXJhY3RlcnMsIHNpemVfdCBsZW5ndGgpOwotICAg IEFMV0FZU19JTkxJTkUgY29uc3QgSWRlbnRpZmllciogbWFrZUlkZW50aWZpZXJTYW1lVHlwZShj b25zdCBVQ2hhciogY2hhcmFjdGVycywgc2l6ZV90IGxlbmd0aCk7CisgICAgQUxXQVlTX0lOTElO RSBjb25zdCBJZGVudGlmaWVyKiBtYWtlUmlnaHRTaXplZElkZW50aWZpZXIoY29uc3QgVUNoYXIq IGNoYXJhY3RlcnMsIHNpemVfdCBsZW5ndGgsIFVDaGFyIG9yQWxsQ2hhcnMpOwogICAgIEFMV0FZ U19JTkxJTkUgY29uc3QgSWRlbnRpZmllciogbWFrZUlkZW50aWZpZXJMQ2hhckZyb21VQ2hhcihj b25zdCBVQ2hhciogY2hhcmFjdGVycywgc2l6ZV90IGxlbmd0aCk7CiAKICAgICBBTFdBWVNfSU5M SU5FIGJvb2wgbGFzdFRva2VuV2FzUmVzdHJLZXl3b3JkKCkgY29uc3Q7CkBAIC0yNDIsMTQgKzI0 MiwxNyBAQCBBTFdBWVNfSU5MSU5FIGNvbnN0IElkZW50aWZpZXIqIExleGVyPFQ+CiB9CiAKIHRl bXBsYXRlIDw+Ci1BTFdBWVNfSU5MSU5FIGNvbnN0IElkZW50aWZpZXIqIExleGVyPExDaGFyPjo6 bWFrZUlkZW50aWZpZXJTYW1lVHlwZShjb25zdCBVQ2hhciogY2hhcmFjdGVycywgc2l6ZV90IGxl bmd0aCkKK0FMV0FZU19JTkxJTkUgY29uc3QgSWRlbnRpZmllciogTGV4ZXI8TENoYXI+OjptYWtl UmlnaHRTaXplZElkZW50aWZpZXIoY29uc3QgVUNoYXIqIGNoYXJhY3RlcnMsIHNpemVfdCBsZW5n dGgsIFVDaGFyKQogewogICAgIHJldHVybiAmbV9hcmVuYS0+bWFrZUlkZW50aWZpZXJMQ2hhckZy b21VQ2hhcihtX2dsb2JhbERhdGEsIGNoYXJhY3RlcnMsIGxlbmd0aCk7CiB9CiAKIHRlbXBsYXRl IDw+Ci1BTFdBWVNfSU5MSU5FIGNvbnN0IElkZW50aWZpZXIqIExleGVyPFVDaGFyPjo6bWFrZUlk ZW50aWZpZXJTYW1lVHlwZShjb25zdCBVQ2hhciogY2hhcmFjdGVycywgc2l6ZV90IGxlbmd0aCkK K0FMV0FZU19JTkxJTkUgY29uc3QgSWRlbnRpZmllciogTGV4ZXI8VUNoYXI+OjptYWtlUmlnaHRT aXplZElkZW50aWZpZXIoY29uc3QgVUNoYXIqIGNoYXJhY3RlcnMsIHNpemVfdCBsZW5ndGgsIFVD aGFyIG9yQWxsQ2hhcnMpCiB7CisgICAgaWYgKCEob3JBbGxDaGFycyAmIH4weGZmKSkKKyAgICAg ICAgcmV0dXJuICZtX2FyZW5hLT5tYWtlSWRlbnRpZmllckxDaGFyRnJvbVVDaGFyKG1fZ2xvYmFs RGF0YSwgY2hhcmFjdGVycywgbGVuZ3RoKTsKKwogICAgIHJldHVybiAmbV9hcmVuYS0+bWFrZUlk ZW50aWZpZXIobV9nbG9iYWxEYXRhLCBjaGFyYWN0ZXJzLCBsZW5ndGgpOwogfQogCg==