Closed Bug 1122160 Opened 5 years ago Closed 5 years ago

support += and -= with DebugOnly variable

Categories

(Core :: MFBT, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla38

People

(Reporter: bkelly, Assigned: bkelly)

References

Details

Attachments

(1 file, 1 obsolete file)

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: nobody → bkelly
Status: NEW → ASSIGNED
Attachment #8549800 - Flags: review?(nfroyd)
Attachment #8549800 - Flags: review?(nfroyd) → review+
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.
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+
Blocks: 940273
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
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
You need to log in before you can comment on or make changes to this bug.