Bug 28428 – Change namespace "ARM" to "ARMRegisters"

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

[Yong Li]

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.

[Yong Li]

Created attachment 35042 [details]
the patch

[Zoltan Herczeg]

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]

(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]

> 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]

(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]

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]

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 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 on attachment 35149 [details]
change X86 and ARM in ARMv7, too

commit-queue bug, sorry.

[Gavin Barraclough]

np, I can land this in a couple of hours if nobody else has before.

[Eric Seidel (no email)]

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)]

All reviewed patches have been landed.  Closing bug.