support += and -= with DebugOnly variable

RESOLVED FIXED in mozilla38

Status

()

Core
MFBT
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: bkelly, Assigned: bkelly)

Tracking

unspecified
mozilla38
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

4 years ago
Currently DebugOnly provides:

  operator=()
  operator++()
  operator--()

It would be nice to support operator+=() and operator-=() as well.

Stylistically I prefer += to ++ outside of for-loops.  I was surprised when DebugOnly += broke in non-DEBUG builds.
(Assignee)

Comment 1

4 years ago
Created attachment 8549800 [details] [diff] [review]
Add operator+=() and operator-=() to DebugOnly (v0)
Assignee: nobody → bkelly
Status: NEW → ASSIGNED
Attachment #8549800 - Flags: review?(nfroyd)
Attachment #8549800 - Flags: review?(nfroyd) → review+
(Assignee)

Comment 2

4 years ago
Actually, when I try to use this in a build I get a warning:

 7:18.26 /srv/mozilla-central/dom/cache/CacheStreamControlParent.cpp:104:22: error: ISO C++ says that th
ese are ambiguous, even though the worst conversion for the first is better than the worst conversion fo
r the second: [-Werror]
 7:18.26 In file included from ../../dist/include/ipc/IPCMessageUtils.h:14:0,
 7:18.26                  from /srv/mozilla-central/obj-x86_64-unknown-linux-gnu-debug/ipc/ipdl/_ipdlheaders/mozilla/dom/cache/PCacheStreamControl.h:12,
 7:18.26                  from /srv/mozilla-central/obj-x86_64-unknown-linux-gnu-debug/ipc/ipdl/_ipdlheaders/mozilla/dom/cache/PCacheStreamControlParent.h:9,
 7:18.26                  from ../../dist/include/mozilla/dom/cache/CacheStreamControlParent.h:10,
 7:18.26                  from /srv/mozilla-central/dom/cache/CacheStreamControlParent.cpp:7:
 7:18.26 ../../dist/include/mozilla/DebugOnly.h:54:14: note: candidate 1: mozilla::DebugOnly<T>& mozilla::DebugOnly<T>::operator+=(const T&) [with T = unsigned int; mozilla::DebugOnly<T> = mozilla::DebugOnly<unsigned int>]
 7:18.26 /srv/mozilla-central/dom/cache/CacheStreamControlParent.cpp:104:22: note: candidate 2: operator+=(unsigned int&, int) <built-in>
Ugh.
(Assignee)

Comment 4

4 years ago
Created attachment 8549842 [details] [diff] [review]
Add operator+=() and operator-=() to DebugOnly (v1)

So the solution is just to provide the operator+=() and operator-=() in non-debug builds.  When DEBUG is defined then the implicit cast operator allows the built-in += and -= to be used.
Attachment #8549800 - Attachment is obsolete: true
Attachment #8549842 - Flags: review?(nfroyd)
Comment on attachment 8549842 [details] [diff] [review]
Add operator+=() and operator-=() to DebugOnly (v1)

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

r=me with the change below:

::: mfbt/DebugOnly.h
@@ +66,5 @@
>    DebugOnly& operator=(const T&) { return *this; }
>    void operator++(int) { }
>    void operator--(int) { }
> +  DebugOnly& operator+=(const T&) { return *this; }
> +  DebugOnly& operator-=(const T&) { return *this; }

Can you add a comment describing why we define these in this arm of the #ifdef, but not in the previous arm?
Attachment #8549842 - Flags: review?(nfroyd) → review+
(Assignee)

Updated

4 years ago
Blocks: 940273
(Assignee)

Comment 7

4 years ago
Apparently the world is not ready for ooperators:

  https://hg.mozilla.org/integration/mozilla-inbound/rev/ef8894863cdf
https://hg.mozilla.org/mozilla-central/rev/7a80f8e49992
https://hg.mozilla.org/mozilla-central/rev/ef8894863cdf
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
You need to log in before you can comment on or make changes to this bug.