Closed Bug 820570 Opened 8 years ago Closed 8 years ago

Move DebugOnly into DebugOnly.h


(Core :: MFBT, defect)

Not set





(Reporter: Waldo, Assigned: Waldo)




(1 file)

Attached patch PatchSplinter Review
I would be very unsurprised if this doesn't rely on bootlegged includes somewhere, but it gets the job done to the point of adding the new header and removing the definitions from Util.h.

Most of the places where I remove a Util.h include, I observed nothing else mozilla:: being used.  I'm not sure whether to be surprised or unsurprised that most people wanting Util.h use it just for DebugOnly.  Probably unsurprised, given the rest of Util.h is rather less commonly useful.
Attachment #691059 - Flags: review?(Ms2ger)
Comment on attachment 691059 [details] [diff] [review]

Review of attachment 691059 [details] [diff] [review]:

::: accessible/src/html/HTMLTableAccessible.cpp
@@ +8,1 @@
>  #include "HTMLTableAccessible.h"

In general, I think Foo.h should be included first in Foo.cpp, to make sure it compiles on its own, but I won't ask you to do that :)

::: dom/bindings/BindingUtils.cpp
@@ +9,2 @@
>  #include <algorithm>
>  #include <stdarg.h>

But I think we're pretty consistent about std headers first, so please do put DebugOnly below these.

::: dom/devicestorage/nsDeviceStorage.cpp
@@ -12,5 @@
>  #include "mozilla/dom/devicestorage/PDeviceStorageRequestChild.h"
>  #include "mozilla/dom/ipc/Blob.h"
>  #include "mozilla/dom/PBrowserChild.h"
>  #include "mozilla/dom/PContentPermissionRequestChild.h"
> -#include "mozilla/Util.h" // DebugOnly

IMO, we should just sort lexicographically, not split on MFBT/non-MFBT.

::: js/src/frontend/TokenStream.h
@@ +12,3 @@
>  /*
>   * JS lexical scanner interface.
>   */

Not after this comment?
Attachment #691059 - Flags: review?(Ms2ger) → review+
I looked back at every change in the patch, even made many of the <>-first changes requested, and it's definitely not the case that we're consistent about <>-first.  :-)  Nor are we really consistent on any particular point.  That goes for JS, too -- we're better about having some sort of ideal, but we only kind of stick to it.

Anyway, I did a certain amount of fixup here, and as I said looked at every file, but I probably missed some stuff and/or didn't change it all quite consistently.  Still, this is an improvement.
Target Milestone: --- → mozilla20
Oh, I think someone's going to need to do a little tweaking to comm-central when this lands:^[^\0]*%24&hitlimit=&tree=comm-central

That comes down to just fixing mailnews/addrbook/src/nsAbView.cpp to include DebugOnly.h, if that comment's correct by doing away with the Util.h include.
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.