1093191 - Make .cpp files in editor/libeditor include their own .h file first

Status: RESOLVED FIXED

(Core :: DOM: Editor, defect)

Description

[Daniel Holbert [:dholbert]]

We have a general best-practice that Foo.cpp's first #include should be for Foo.h (if Foo.h exists).

This lets us verify that Foo.h correctly forward-declares all the types that it uses.  (If it doesn't, then Foo.cpp will fail to compile.

Filing this bug on fixing up .cpp files in editor/libeditor to conform to this.

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: part 1: drop unneeded stdio.h includes from 3 files

First: when bumping #includes up for this bug, I noticed that several of these files start with...
> #include <stdio.h>                      // for printf
...and yet don't use printf at all.

So, this patch drops those includes.

Comment 2

[Daniel Holbert [:dholbert]]

Attachment: part 2: add missing forward-decls / #includes

Unsurprisingly, the "main" patch here (which I'll post next) caused build failures, because it caught some missing forward-decls / includes in the .h files.

This patch adds those missing forward-decls / includes, so that we can continue compiling.

(Note: I built with "ac_add_options --disable-unified-compilation" to catch these, since otherwise, unified compilation lets us cheat & have the things we need already declared/defined by previous .cpp files.)

Comment 3

[Daniel Holbert [:dholbert]]

Comment on attachment 8516074 [details] [diff] [review]
part 2: add missing forward-decls / #includes

>+++ b/editor/libeditor/nsHTMLEditUtils.h
> #ifndef nsHTMLEditUtils_h__
> #define nsHTMLEditUtils_h__
> 
>+#include "mozilla/Types.h"
> 

(In case it's not clear, this ^ include is to give us "int32_t", which this header uses in some function-signatures lower down.)

Comment 4

[Daniel Holbert [:dholbert]]

Attachment: part 3: make foo.cpp include foo.h first

...and here's the main patch here -- making each .cpp file #include its own .h file first.

Comment 5

[Daniel Holbert [:dholbert]]

(FWIW, I looked for candidate .cpp files to fix here with the following shell command...

for cppfile in *cpp; do hfile=`echo $cppfile | sed "s/.cpp/.h/"`; echo; echo "***$cppfile***"; grep -B4 $hfile $cppfile; done

...and I skimmed the output for chunks with other #includes in grep's before-context from "-B4".)

Comment 6

[Daniel Holbert [:dholbert]]

Try run: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=3d11f4026885

Comment 7

[Daniel Holbert [:dholbert]]

(Try run is green.)

Comment 8

[(no longer active)]

(In reply to Daniel Holbert [:dholbert] from comment #3)
> Comment on attachment 8516074 [details] [diff] [review]
> part 2: add missing forward-decls / #includes
> 
> >+++ b/editor/libeditor/nsHTMLEditUtils.h
> > #ifndef nsHTMLEditUtils_h__
> > #define nsHTMLEditUtils_h__
> > 
> >+#include "mozilla/Types.h"
> > 
> 
> (In case it's not clear, this ^ include is to give us "int32_t", which this
> header uses in some function-signatures lower down.)

You could just include <stdint.h> for that, right?

Comment 9

[Daniel Holbert [:dholbert]]

(In reply to :Ehsan Akhgari (not reading bugmail, needinfo? me!) from comment #8)
> You could just include <stdint.h> for that, right?

Hmm, looks like it. Not sure why I'd thought I needed mfbt's Types.h for that. I'll sanity-check that w/ Try, and then land if all is well. Thanks for the review!

Comment 10

[Daniel Holbert [:dholbert]]

Try run with that fixed: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=19222b165039

Comment 11

[Daniel Holbert [:dholbert]]

Landed:
 https://hg.mozilla.org/integration/mozilla-inbound/rev/3361018e53e5
 https://hg.mozilla.org/integration/mozilla-inbound/rev/f263abe1e116
 https://hg.mozilla.org/integration/mozilla-inbound/rev/debcb0ca1fec

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/3361018e53e5
https://hg.mozilla.org/mozilla-central/rev/f263abe1e116
https://hg.mozilla.org/mozilla-central/rev/debcb0ca1fec