ASCIICType.h is just a bunch of function copied for every type. I have heard of that template thingy that is good at doing that ;)
Created attachment 119703 [details] Patch
I was also curious why we need to support so many types and not just UChar for every function... It is because of the JSC::Yarr() and JSC::lexer(), which use various types for the characters. This could probably be fixed but I preferred to limit the scope of this particular change.
Comment on attachment 119703 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=119703&action=review Great idea! > Source/JavaScriptCore/ChangeLog:8 > + The functions were sharing similar code and were defined for the various input type. type -> types > Source/JavaScriptCore/ChangeLog:9 > + Use template instead to avoid code duplication. template -> templates > Source/JavaScriptCore/wtf/ASCIICType.h:98 > + character count > + --------- ----- > + non-spaces 689383 > + 20 space 294720 > + 0A \n 89059 > + 09 \t 28320 > + 0D \r 0 > + 0C \f 0 > + 0B \v 0 > + */ Looks like you're breaking the alignment here a bit.
Committed r103144: <http://trac.webkit.org/changeset/103144>
Reopen, I am not finished here :)
Created attachment 119715 [details] Patch
Comment on attachment 119715 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=119715&action=review > Source/JavaScriptCore/wtf/ASCIICType.h:58 > + return (c >= '0') && (c <= '9'); You don't need parentheses here. NABD.
Committed r103202: <http://trac.webkit.org/changeset/103202>
(In reply to comment #7) > > Source/JavaScriptCore/wtf/ASCIICType.h:58 > > + return (c >= '0') && (c <= '9'); > > You don't need parentheses here. NABD. I know, but I like the parentheses :) The helmet is not mandatory when rollerblading(*), but it is safer ;) (*) restrictions may apply depending on your country. The lack of helmet can have side effects including nausea, headache or death. If you feel any discomfort, immediately consult a doctor.
The first patch is marked as obsolete. Was it rolled out? If it's in the tree, it should not be obsolete, as otherwise it's too hard to see what changes the bug included.
(In reply to comment #10) > The first patch is marked as obsolete. Was it rolled out? If it's in the tree, it should not be obsolete, as otherwise it's too hard to see what changes the bug included. You are right. I used webkit-patch without thinking about that.