1618038 - Define inequality operators on wrapper classes in their namespaces, not in the global namespace

Status: RESOLVED FIXED

(Core :: JavaScript Engine, task, P2)

Description

[Jeff Walden [:Waldo]]

Idiomatic C++ defines operator== and such in the namespace of the arguments, because ADL only looks in the innermost namespace. So if the context of a comparison has any operator== visible in it, it'll shadow the global operator== and you'll get no-valid-overload compile errors. We should do this for our various wrapper classes.

(This will also avoid bz [and everyone else] ever running into this problem again, for code that does comparisons inside namespace mozilla or in a place with a using namespace mozilla; in it. It is a Very Good Use Of Time for everyone to eliminate such recurring sadtimes.)

This gets a bit tricky because our wrapper classes don't all reside in the same namespace, but this is work-aroundable by defining the operators once in one namespace, then using them into each wrapper-enclosing namespace.

If comments fail to adequately clarify the setup here and why it is as it is, tell me and I'll futz with them. :-) This is grody, and saving the next reader time trying to understand stuff is definitely worth it.

Comment 1

[Jeff Walden [:Waldo]]

Attachment: Bug 1618038 - Move |MaybeRooted|, |FakeRooted|, and |FakeMutableHandle| to a fresh header. r=sfink

Comment 2

[Jeff Walden [:Waldo]]

Attachment: Bug 1618038 - Add some |if constexpr| to eliminate |to{,Mutable}Handle| functions on |FakeRooted|. r=sfink

Depends on D64236

Comment 3

[Jeff Walden [:Waldo]]

Attachment: Bug 1618038 - Implement support for wrapper-class comparisons in the innermost namespace containing each wrapper class, as ADL intended. Also simplify the operator implementations in certain minor ways. r=sfink!

Depends on D64239

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2404d662be36
Move |MaybeRooted|, |FakeRooted|, and |FakeMutableHandle| to a fresh header.  r=sfink
https://hg.mozilla.org/integration/autoland/rev/e627b9d49bff
Add some |if constexpr| to eliminate |to{,Mutable}Handle| functions on |FakeRooted|.  r=sfink

Comment 5

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/2404d662be36
https://hg.mozilla.org/mozilla-central/rev/e627b9d49bff

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f6898bd931c5
Implement support for wrapper-class comparisons in the innermost namespace containing each wrapper class, as ADL intended.  Also simplify the operator implementations in certain minor ways.  r=sfink

Comment 7

[Narcis Beleuzu [:NarcisB]]

Backed out for SM bustages on AbstractScope.h

Backout link: https://hg.mozilla.org/integration/autoland/rev/dc81efd516c4325474b4eab3e70e36395c2cb155
Log link: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=292344597&repo=autoland&lineNumber=715

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b855d9239a65
Implement support for wrapper-class comparisons in the innermost namespace containing each wrapper class, as ADL intended.  Also simplify the operator implementations in certain minor ways.  r=sfink

Comment 9

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/b855d9239a65