Closed Bug 1118486 Opened 9 years ago Closed 9 years ago

Remove MOZ_DELETE macro and use `= delete` directly

Categories

(Core :: MFBT, defect)

All
Windows 8
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla37

People

(Reporter: cpeterson, Assigned: ehsan.akhgari)

References

Details

Attachments

(2 files)

Bug 1117820 removed support for MSVC 2012, so we should be able to remove the MOZ_DELETE macro and use `= delete` directly.
Most of this patch (with the exception of mfbt/Attributes.h and
dom/bindings/Codegen.py) was generated by the following bash script:


function convert() {
echo "Converting $1 to $2..."
find . ! -wholename "*nsprpub*" \
       ! -wholename "*security/nss*" \
       ! -wholename "*/.hg*" \
       ! -wholename "*/.git*" \
       ! -wholename "obj-*" \
         -type f \
      \( -iname "*.cpp" \
         -o -iname "*.h" \
         -o -iname "*.cc" \
         -o -iname "*.idl" \
         -o -iname "*.ipdl" \
         -o -iname "*.ipdlh" \
         -o -iname "*.mm" \) | \
    xargs -n 1 sed -i -e "s/\b$1\b/$2/g"
}

convert MOZ_DELETE '= delete'
Attachment #8544872 - Flags: review?(jwalden+bmo)
Assignee: nobody → ehsan
I was about to say you should do the same in comm-central, but it turns out there is no use of MOZ_DELETE in comm-central.
>-#include "mozilla/Attributes.h"         // for MOZ_DELETE, MOZ_STACK_CLASS
>+#include "mozilla/Attributes.h"         // for = delete, MOZ_STACK_CLASS

We don't need any header files for |= delete|. Also we could remove the #include from some files.
Comment on attachment 8544872 [details] [diff] [review]
Remove MOZ_DELETE macro and use  directly

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

Seems like you should land as much of this patch as is trivially doable.  The parts where I have issues, plus the ultimate mfbt/Attributes.h changes, you can mop up in a far-smaller followup patch, since most of these changes are totally independent of each otehr.

::: gfx/layers/basic/BasicLayersImpl.h
@@ +9,5 @@
>  #include "BasicImplData.h"              // for BasicImplData
>  #include "BasicLayers.h"                // for BasicLayerManager
>  #include "ReadbackLayer.h"              // for ReadbackLayer
>  #include "gfxContext.h"                 // for gfxContext, etc
> +#include "mozilla/Attributes.h"         // for = delete, MOZ_STACK_CLASS

Yeah, change this (manually) to just MOZ_STACK_CLASS now.

::: gfx/layers/composite/AsyncCompositionManager.h
@@ +7,5 @@
>  #define GFX_ASYNCCOMPOSITIONMANAGER_H
>  
>  #include "Units.h"                      // for ScreenPoint, etc
>  #include "mozilla/layers/LayerManagerComposite.h"  // for LayerManagerComposite
> +#include "mozilla/Attributes.h"         // for = delete, MOZ_FINAL, etc

And this, in similar fashion.

::: mfbt/Scoped.h
@@ +216,5 @@
>              MOZ_GUARD_OBJECT_NOTIFIER_PARAM_TO_PARENT)                        \
>    {}                                                                          \
>  private:                                                                      \
> +  explicit name(name&) = delete;                                            \
> +  name& operator=(name&) = delete;                                          \

These need the \ realigned.

::: mfbt/decimal/Decimal.h
@@ +51,5 @@
>  // To use WTF_MAKE_FAST_ALLOCATED we'd need:
>  // http://src.chromium.org/viewvc/blink/trunk/Source/wtf/FastMalloc.h
>  // Since we don't allocate Decimal objects, no need.
>  #define WTF_MAKE_FAST_ALLOCATED \
> +  void ignore_this_dummy_method() = delete

This requires changes to mfbt/decimal/to-moz-dependencies.h, possibly.  Remove all the mfbt/decimal changes and put them in a followup patch.
Attachment #8544872 - Flags: review?(jwalden+bmo) → review+
Attachment #8546390 - Flags: review?(jwalden+bmo) → review+
https://hg.mozilla.org/mozilla-central/rev/a0b1e0b8a80a
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
You need to log in before you can comment on or make changes to this bug.