[webkit-reviews] review granted: [Bug 56041] RexExp constructor should only accept flags "gim" : [Attachment 85213] The patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed Mar 9 12:28:51 PST 2011
Darin Adler <darin at apple.com> has granted Gavin Barraclough
<barraclough at apple.com>'s request for review:
Bug 56041: RexExp constructor should only accept flags "gim"
https://bugs.webkit.org/show_bug.cgi?id=56041
Attachment 85213: The patch
https://bugs.webkit.org/attachment.cgi?id=85213&action=review
------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=85213&action=review
> Source/JavaScriptCore/runtime/RegExp.h:27
> +#include "RegExpKey.h"
Seems a little awkward to have to include RegExpKey here to get this flags
enum.
>>> Source/JavaScriptCore/runtime/RegExp.h:40
>>> + static PassRefPtr<RegExp> create(JSGlobalData* globalData, const
UString& pattern, RegExpFlags flags);
>>
>> The parameter name "globalData" adds no information, so it should be
removed. [readability/parameter_name] [5]
>
> The parameter name "flags" adds no information, so it should be removed.
[readability/parameter_name] [5]
I agree with the stylebot.
>>> Source/JavaScriptCore/runtime/RegExp.h:60
>>> + RegExp(JSGlobalData* globalData, const UString& pattern,
RegExpFlags flags);
>>
>> The parameter name "globalData" adds no information, so it should be
removed. [readability/parameter_name] [5]
>
> The parameter name "flags" adds no information, so it should be removed.
[readability/parameter_name] [5]
I agree with the stylebot.
>> Source/JavaScriptCore/runtime/RegExpCache.h:44
>> + PassRefPtr<RegExp> lookupOrCreate(const UString& patternString,
RegExpFlags flags);
>
> The parameter name "flags" adds no information, so it should be removed.
[readability/parameter_name] [5]
I agree with the stylebot.
>>> Source/JavaScriptCore/runtime/RegExpCache.h:45
>>> + PassRefPtr<RegExp> create(const UString& patternString, RegExpFlags
flags, RegExpCacheMap::iterator iterator);
>>
>> The parameter name "flags" adds no information, so it should be removed.
[readability/parameter_name] [5]
>
> The parameter name "iterator" adds no information, so it should be removed.
[readability/parameter_name] [5]
I agree with the stylebot.
> Source/JavaScriptCore/runtime/RegExpConstructor.cpp:313
> + flags = regExpFlags(arg1.toString(exec));
Should return here if toString raised an exception rather than continuing with
NoFlags.
> Source/JavaScriptCore/runtime/RegExpKey.h:36
> +enum RegExpFlags {
Flag bits are not quite an enum since they can be enum values or'ed together.
In some places we’ve used a separate integral typedef for this. I guess this is
OK.
> Source/JavaScriptCore/runtime/RegExpPrototype.cpp:98
> + flags = regExpFlags(arg1.toString(exec));
Should return here if toString raised an exception rather than continuing with
NoFlags.
More information about the webkit-reviews
mailing list