Bug 28428 - Change namespace "ARM" to "ARMRegisters"
Summary: Change namespace "ARM" to "ARMRegisters"
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-08-18 07:36 PDT by Yong Li
Modified: 2009-08-19 18:39 PDT (History)
5 users (show)

See Also:


Attachments
the patch (58.07 KB, patch)
2009-08-18 08:28 PDT, Yong Li
no flags Details | Formatted Diff | Diff
change X86 and ARM in ARMv7, too (86.54 KB, patch)
2009-08-19 15:28 PDT, Yong Li
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yong Li 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.
Comment 1 Yong Li 2009-08-18 08:28:15 PDT
Created attachment 35042 [details]
the patch
Comment 2 Zoltan Herczeg 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.
Comment 3 Yong Li 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?
Comment 4 Zoltan Herczeg 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 :)
Comment 5 Yong Li 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!
Comment 6 Gavin Barraclough 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.
Comment 7 Yong Li 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
Comment 8 Eric Seidel (no email) 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
Comment 9 Eric Seidel (no email) 2009-08-19 16:52:28 PDT
Comment on attachment 35149 [details]
change X86 and ARM in ARMv7, too

commit-queue bug, sorry.
Comment 10 Gavin Barraclough 2009-08-19 16:53:19 PDT
np, I can land this in a couple of hours if nobody else has before.
Comment 11 Eric Seidel (no email) 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>
Comment 12 Eric Seidel (no email) 2009-08-19 17:02:37 PDT
All reviewed patches have been landed.  Closing bug.