Bug 102572 – Don't blind all the things.

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Oliver Hunt]

Don't blind all the things.

[Oliver Hunt]

Created attachment 176046 [details]
Patch

[Darin Adler]

Comment on attachment 176046 [details]
Patch

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

> Source/JavaScriptCore/ChangeLog:12
> +        (MacroAssembler):

Please delete bogus lines like this from change log.

> Source/JavaScriptCore/assembler/MacroAssembler.h:843
> +    bool shouldConsiderBlinding()

Should be a static member function?

> Source/JavaScriptCore/assembler/MacroAssembler.h:847
>      bool shouldBlindDouble(double value)

Should be a static member function?

> Source/JavaScriptCore/assembler/MacroAssembler.h:895
> +            if (~value <= 0xff)
> +                return false;

No explanation of this change in the change log.

> Source/JavaScriptCore/assembler/MacroAssembler.h:954
> +            if (~value <= 0xff)
> +                return false;

No explanation of this change in the change log.

> Source/JavaScriptCore/assembler/MacroAssembler.h:1089
> +            if (~value <= 0xff)
> +                return false;

No explanation of this change in the change log.

[Gavin Barraclough]

Comment on attachment 176046 [details]
Patch

r=me, please add changelog comment re blinding small negative value.

[Oliver Hunt]

Committed r135757: <http://trac.webkit.org/changeset/135757>

[Darin Adler]

Comment on attachment 176046 [details]
Patch

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

>> Source/JavaScriptCore/assembler/MacroAssembler.h:843
>> +    bool shouldConsiderBlinding()
> 
> Should be a static member function?

Why is this a non-static member function instead of a static one?

>> Source/JavaScriptCore/assembler/MacroAssembler.h:847
>>      bool shouldBlindDouble(double value)
> 
> Should be a static member function?

Why is this a non-static member function instead of a static one?

[Oliver Hunt]

(In reply to comment #5)
> (From update of attachment 176046 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=176046&action=review
> 
> >> Source/JavaScriptCore/assembler/MacroAssembler.h:843
> >> +    bool shouldConsiderBlinding()
> > 
> > Should be a static member function?
> 
> Why is this a non-static member function instead of a static one?
> 
> >> Source/JavaScriptCore/assembler/MacroAssembler.h:847
> >>      bool shouldBlindDouble(double value)
> > 
> > Should be a static member function?
> 
> Why is this a non-static member function instead of a static one?

Because they use member functions