1232418 - Allow equality comparison of Tuple types

Status: RESOLVED FIXED

(Core :: MFBT, defect)

Description

[Terrence Cole [:terrence]]

Attachment: teach_Tuple_operatorEq-v0.diff

As discussed on IRC before the work week.

Comment 1

[Jeff Walden [:Waldo]]

Comment on attachment 8698146 [details] [diff] [review]
teach_Tuple_operatorEq-v0.diff

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

I'm a bit confused why Head/Tail are publicly exposed, but that's aside from this bug/patch.  (And it looks like there might be other things publicly expose that shouldn't be as well.  Still a separate bug.)

::: mfbt/Tuple.h
@@ +186,5 @@
>      Head(*this) = Move(Head(aOther));
>      Tail(*this) = Move(Tail(aOther));
>      return *this;
>    }
> +  bool operator==(const TupleImpl& aOther) const

Can this method and the empty-case operator== above be protected instead of public?  Tuple::operator== is the only one that we really want exposed.

Comment 2

[Terrence Cole [:terrence]]

Attachment: build_output.txt

It seems that no, we cannot make those protected. The failing build output is inscrutable in the way that only C++ template errors can be inscrutable, but what I think it comes down to is: for TupleImpl<n, HeadT, TailT...>, we are derived from TupleImpl<n+1, TailT...>, however, Head and Tail return HeadT and TailT refs which are not the same as Base. They are also TupleImpls, but are not on our ancestor chain, so we don't have protected access there. We might be able to friend ourself to HeadT and TailT? I'm not very confident in C++'s friendship model, and I've rarely had luck with template friends.

Comment 3

[Terrence Cole [:terrence]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=d57231f1c4a4

Comment 4

[Terrence Cole [:terrence]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b668ab1846b0170427ff3a3e74e41c706a4464c1
Bug 1232418 - Allow mozilla::Tuple to support equality comparison; r=Waldo

Comment 5

[Carsten Book [:Tomcat]]

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