672313 - class declared in one namespace and defined in another

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Rafael Ávila de Espíndola (:espindola) (not reading bugmail)]

Attachment: define Proxy in the namespace it is declared

The class Proxy is declared in mozilla::dom::workers::xhr, but is defined outside of any namespace in XMLHttpRequestPrivate.cpp.

Clang complains about this. It is possible that this is clang bug (http://llvm.org/pr10393), but it IMHO the code is easier to understand anyway if the class is explicitly defined in the same namespace it is declared. The attached does that.

Comment 1

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Comment on attachment 546598 [details] [diff] [review]
define Proxy in the namespace it is declared

Review of attachment 546598 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with the following changes:

::: dom/workers/XMLHttpRequestPrivate.cpp
@@ +61,5 @@
>  #include "XMLHttpRequest.h"
>  
> +namespace mozilla {
> +namespace dom {
> +namespace workers {

Please use BEGIN_WORKERS_NAMESPACE and END_WORKERS_NAMESPACE macros for this.

@@ +189,5 @@
>  
> +}
> +}
> +}
> +}

Please make this:

  } // namespace xhr

Comment 2

[Rafael Ávila de Espíndola (:espindola) (not reading bugmail)]

Attachment: updated patch

Comment 3

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

This was backed out. bent, it would probably be best if you merged this into the right patch.

Comment 4

[(no longer active)]

Ben, it would be great if we can get this second patch relanded ASAP.  Right now, Firefox trunk fails to build with clang.  :(

Comment 5

[Rafael Ávila de Espíndola (:espindola) (not reading bugmail)]

The original patch applies cleanly. Can it be checked in?

Comment 6

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Comment on attachment 546655 [details] [diff] [review]
updated patch

If the reviewer gave you r+ the first time they trust you to make the requested changes on your own.

Comment 7

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Sorry, I wasn't CC'd, and I forgot :(

Comment 8

[(no longer active)]

Pushed to inbound.

Comment 9

[Marco Bonardo [:mak]]

http://hg.mozilla.org/mozilla-central/rev/5ec41b0085be