Bug 156941 - Renaming SpecInt32, SpecInt52, MachineInt to SpecInt32Only, SpecInt52Only, AnyInt.
Summary: Renaming SpecInt32, SpecInt52, MachineInt to SpecInt32Only, SpecInt52Only, An...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Local Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mark Lam
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-04-22 17:28 PDT by Mark Lam
Modified: 2016-04-25 10:49 PDT (History)
8 users (show)

See Also:


Attachments
proposed patch. (109.11 KB, patch)
2016-04-22 17:34 PDT, Mark Lam
no flags Details | Formatted Diff | Diff
proposed patch w/ Fil's feedback applied. (115.25 KB, patch)
2016-04-25 10:35 PDT, Mark Lam
fpizlo: review+
Details | Formatted Diff | Diff
Diff of patches (last vs current) for your convenience. (13.31 KB, text/plain)
2016-04-25 10:36 PDT, Mark Lam
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Lam 2016-04-22 17:28:49 PDT
While looking at https://bugs.webkit.org/show_bug.cgi?id=153431, it was decided that SpecInt32Only, SpecInt52Only, and AnyInt would be better names for SpecInt32, SpecInt52, and MachineInt.  Let's do a bulk rename.
Comment 1 Mark Lam 2016-04-22 17:34:39 PDT
Created attachment 277120 [details]
proposed patch.
Comment 2 WebKit Commit Bot 2016-04-22 17:36:58 PDT
Attachment 277120 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/bytecode/SpeculatedType.cpp:185:  Multi line control clauses should use braces.  [whitespace/braces] [4]
Total errors found: 1 in 37 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Filip Pizlo 2016-04-25 10:15:11 PDT
Comment on attachment 277120 [details]
proposed patch.

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

> Source/JavaScriptCore/bytecode/SpeculatedType.h:71
> +static const SpeculatedType SpecAnyInt             = SpecInt32Only | SpecInt52Only; // It's something that we can do machine int arithmetic on.

It's a shame that this is called AnyInt while we have:

> Source/JavaScriptCore/bytecode/SpeculatedType.h:72
>  static const SpeculatedType SpecInt52AsDouble      = 1u << 24; // It's definitely an Int52 and it's inside a double.

Under the new nomenclature, this should be called SpecAnyIntAsDouble.

> Source/JavaScriptCore/bytecode/SpeculatedType.h:73
> +static const SpeculatedType SpecInteger            = SpecAnyInt | SpecInt52AsDouble; // It's definitely some kind of integer.

And this.

I think this is why the other one was called MachineInt.  Looking at the code, it seems that SpecInteger is used only in one place.  We could just remove SpecInteger and replace its one use with "SpecAnyInt | SpecAnyIntAsDouble"
Comment 4 Mark Lam 2016-04-25 10:35:24 PDT
Created attachment 277258 [details]
proposed patch w/ Fil's feedback applied.
Comment 5 Mark Lam 2016-04-25 10:36:19 PDT
Created attachment 277259 [details]
Diff of patches (last vs current) for your convenience.
Comment 6 WebKit Commit Bot 2016-04-25 10:36:30 PDT
Attachment 277258 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/bytecode/SpeculatedType.cpp:185:  Multi line control clauses should use braces.  [whitespace/braces] [4]
Total errors found: 1 in 37 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 7 Mark Lam 2016-04-25 10:49:07 PDT
Thanks for the review.  Landed in r200034: <http://trac.webkit.org/r200034>.