Bug 28428

Summary: Change namespace "ARM" to "ARMRegisters"
Product: WebKit Reporter: Yong Li <yong.li.webkit>
Component: JavaScriptCoreAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: barraclough, eric, gyuyoung, staikos, zherczeg
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
the patch
none
change X86 and ARM in ARMv7, too none

Yong Li
Reported 2009-08-18 07:36:35 PDT
namespace name "ARM" conflicts with macro "ARM" defined by VS. "ARM" is not a good name for the namespace. Candidates: ARMJIT ARMJit ARMArchitecture ARMitecture ARMRegisters ARMRegisters may be the best because this namespace is used only for listing registers so far.
Attachments
the patch (58.07 KB, patch)
2009-08-18 08:28 PDT, Yong Li 		no flags
DetailsFormatted DiffDiff
change X86 and ARM in ARMv7, too (86.54 KB, patch)
2009-08-19 15:28 PDT, Yong Li 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Yong Li
Comment 1 2009-08-18 08:28:15 PDT
Created attachment 35042 [details] the patch
Zoltan Herczeg
Comment 2 2009-08-19 08:20:36 PDT
Only if X86 and the other ARM (ARMv7) namespaces are renamed as well. Otherwise the generic ARM port will break the porting rules and others will not like this change.
Yong Li
Comment 3 2009-08-19 08:28:45 PDT
(In reply to comment #2) > Only if X86 and the other ARM (ARMv7) namespaces are renamed as well. Otherwise > the generic ARM port will break the porting rules and others will not like this > change. Hm.. Do you think it's good to change all these namespace names?
Zoltan Herczeg
Comment 4 2009-08-19 08:48:23 PDT
> Hm.. Do you think it's good to change all these namespace names? Changing it only for one port just breaks the rules. (I am actually not a fond of these rules. Several of our optimizations are rejected because of them.) Please do it for all ports if you really want this patch to go through. Perhaps it would be good to ask Gavin first, which name he prefers. You could save a lot of trouble for yourself :)
Yong Li
Comment 5 2009-08-19 09:06:13 PDT
(In reply to comment #4) > > Hm.. Do you think it's good to change all these namespace names? > > Changing it only for one port just breaks the rules. (I am actually not a fond > of these rules. Several of our optimizations are rejected because of them.) > > Please do it for all ports if you really want this patch to go through. Perhaps > it would be good to ask Gavin first, which name he prefers. You could save a > lot of trouble for yourself :) Thanks!
Gavin Barraclough
Comment 6 2009-08-19 15:14:36 PDT
As commented in IRC, Zoltan is right, where there is no good reason for the JIT to diverge we like to avoid doing so, so renaming X86 to X86Registers would make sense.
Yong Li
Comment 7 2009-08-19 15:28:29 PDT
Created attachment 35149 [details] change X86 and ARM in ARMv7, too 1. change namespace X86 to X86Registers 2. change namespace ARM in ARMv7 files
Eric Seidel (no email)
Comment 8 2009-08-19 16:51:42 PDT
Comment on attachment 35149 [details] change X86 and ARM in ARMv7, too Rejecting patch 35149 from commit-queue. This patch will require manual commit. Failed to run "['git', 'svn', 'rebase']" exit_code: 1 cwd: None
Eric Seidel (no email)
Comment 9 2009-08-19 16:52:28 PDT
Comment on attachment 35149 [details] change X86 and ARM in ARMv7, too commit-queue bug, sorry.
Gavin Barraclough
Comment 10 2009-08-19 16:53:19 PDT
np, I can land this in a couple of hours if nobody else has before.
Eric Seidel (no email)
Comment 11 2009-08-19 17:02:31 PDT
Comment on attachment 35149 [details] change X86 and ARM in ARMv7, too Clearing flags on attachment: 35149 Committed r47530: <http://trac.webkit.org/changeset/47530>
Eric Seidel (no email)
Comment 12 2009-08-19 17:02:37 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.